https://github.com/denzor200 updated https://github.com/llvm/llvm-project/pull/181570
>From ce04b6476fbb5b2e66a439f407d4e335616d4aff Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Tue, 2 Dec 2025 03:24:44 +0300 Subject: [PATCH 01/14] Initial implementation by DeepSeek --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/SmartPtrInitializationCheck.cpp | 219 ++++++++++++++++++ .../bugprone/SmartPtrInitializationCheck.h | 35 +++ clang-tools-extra/docs/ReleaseNotes.rst | 5 + .../bugprone/smart-ptr-initialization.rst | 6 + .../docs/clang-tidy/checks/list.rst | 19 +- .../bugprone/smart-ptr-initialization.cpp | 145 ++++++++++++ 8 files changed, 422 insertions(+), 11 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 6859dc97c112a0..fcadcdf2b76252 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -75,6 +75,7 @@ #include "SignedCharMisuseCheck.h" #include "SizeofContainerCheck.h" #include "SizeofExpressionCheck.h" +#include "SmartPtrInitializationCheck.h" #include "SpuriouslyWakeUpFunctionsCheck.h" #include "StandaloneEmptyCheck.h" #include "StdNamespaceModificationCheck.h" @@ -176,6 +177,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-incorrect-enable-if"); CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>( "bugprone-incorrect-enable-shared-from-this"); + CheckFactories.registerCheck<SmartPtrInitializationCheck>( + "bugprone-smart-ptr-initialization"); CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>( "bugprone-unintended-char-ostream-output"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index db1256d91d311f..e46edc5bafea21 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -37,6 +37,7 @@ add_clang_library(clangTidyBugproneModule STATIC IncorrectEnableIfCheck.cpp IncorrectEnableSharedFromThisCheck.cpp InvalidEnumDefaultInitializationCheck.cpp + SmartPtrInitializationCheck.cpp UnintendedCharOstreamOutputCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp new file mode 100644 index 00000000000000..392c193918be04 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -0,0 +1,219 @@ +//===----------------------------------------------------------------------===// +// +// 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 "SmartPtrInitializationCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" +#include "clang/Basic/Diagnostic.h" +#include "clang/Lex/Lexer.h" + +using namespace clang; +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +namespace { + +AST_MATCHER(Expr, isNewExpression) { + return isa<CXXNewExpr>(Node); +} + +AST_MATCHER(Expr, isReleaseCall) { + const auto *call = dyn_cast<CallExpr>(&Node); + if (!call) + return false; + + const auto *method = dyn_cast<CXXMemberCallExpr>(call); + if (!method) + return false; + + const auto *member = method->getMethodDecl(); + if (!member) + return false; + + return member->getName() == "release"; +} + +AST_MATCHER(Expr, isMakeUniqueOrSharedCall) { + const auto *call = dyn_cast<CallExpr>(&Node); + if (!call) + return false; + + const auto *callee = call->getDirectCallee(); + if (!callee) + return false; + + StringRef name = callee->getName(); + if (name != "make_unique" && name != "make_shared") + return false; + + // Check if it's in std namespace by checking the qualified name + std::string qualifiedName = callee->getQualifiedNameAsString(); + return qualifiedName == "std::make_unique" || qualifiedName == "std::make_shared"; +} + +AST_MATCHER(CXXConstructExpr, hasCustomDeleter) { + if (Node.getNumArgs() < 2) + return false; + + // Check if the second argument is a deleter + const Expr *deleterArg = Node.getArg(1); + if (!deleterArg) + return false; + + // Check if this is a smart pointer construction with custom deleter + const auto *record = Node.getConstructor()->getParent(); + if (!record) + return false; + + std::string typeName = record->getQualifiedNameAsString(); + return typeName == "std::shared_ptr" || typeName == "std::unique_ptr"; +} + +AST_MATCHER(CallExpr, isResetWithCustomDeleter) { + const auto *memberCall = dyn_cast<CXXMemberCallExpr>(&Node); + if (!memberCall) + return false; + + const auto *method = memberCall->getMethodDecl(); + if (!method || method->getName() != "reset") + return false; + + if (Node.getNumArgs() < 2) + return false; + + const auto *record = method->getParent(); + if (!record) + return false; + + std::string typeName = record->getQualifiedNameAsString(); + return typeName == "std::shared_ptr" || typeName == "std::unique_ptr"; +} + +} // namespace + +void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { + // Matcher for smart pointer constructors + auto smartPtrConstructorMatcher = cxxConstructExpr( + hasDeclaration( + cxxConstructorDecl(ofClass(hasAnyName("std::shared_ptr", + "std::unique_ptr")))), + hasArgument(0, expr().bind("pointer-arg")), + unless(hasCustomDeleter()), + unless(hasArgument(0, isNewExpression())), + unless(hasArgument(0, isReleaseCall())), + unless(hasArgument(0, isMakeUniqueOrSharedCall())) + ).bind("constructor"); + + // Matcher for reset() calls + auto resetCallMatcher = cxxMemberCallExpr( + on(hasType(cxxRecordDecl(hasAnyName("std::shared_ptr", "std::unique_ptr")))), + callee(cxxMethodDecl(hasName("reset"))), + hasArgument(0, expr().bind("pointer-arg")), + unless(isResetWithCustomDeleter()), + unless(hasArgument(0, isNewExpression())), + unless(hasArgument(0, isReleaseCall())), + unless(hasArgument(0, isMakeUniqueOrSharedCall())) + ).bind("reset-call"); + + Finder->addMatcher(smartPtrConstructorMatcher, this); + Finder->addMatcher(resetCallMatcher, this); +} + +void SmartPtrInitializationCheck::check(const MatchFinder::MatchResult &Result) { + const auto *pointerArg = Result.Nodes.getNodeAs<Expr>("pointer-arg"); + if (!pointerArg) + return; + + // Skip if the pointer is a null pointer + if (pointerArg->isNullPointerConstant(*Result.Context, + Expr::NPC_ValueDependentIsNotNull)) + return; + + // Check if the expression is a call to a function returning a pointer + bool isFunctionReturn = false; + if (const auto *call = dyn_cast<CallExpr>(pointerArg)) { + if (call->getDirectCallee()) { + isFunctionReturn = true; + } + } + + // Check if it's taking address of something + bool isAddressOf = isa<UnaryOperator>(pointerArg) && + cast<UnaryOperator>(pointerArg)->getOpcode() == UO_AddrOf; + + // Check if it's getting pointer from reference + const Expr *innerExpr = pointerArg->IgnoreParenCasts(); + if (const auto *unaryOp = dyn_cast<UnaryOperator>(innerExpr)) { + if (unaryOp->getOpcode() == UO_AddrOf) { + isAddressOf = true; + } + } + + // Also check for member expressions that might return references + if (const auto *memberExpr = dyn_cast<MemberExpr>(innerExpr)) { + if (memberExpr->isArrow()) { + // arrow operator returns pointer, not reference + isAddressOf = false; + } + } + + if (isFunctionReturn || isAddressOf) { + std::string message; + const SourceLocation loc = pointerArg->getBeginLoc(); + + if (const auto *constructor = + Result.Nodes.getNodeAs<CXXConstructExpr>("constructor")) { + const auto *decl = constructor->getConstructor(); + if (decl) { + const auto *record = decl->getParent(); + if (record) { + std::string typeName = record->getQualifiedNameAsString(); + message = "passing a raw pointer '" + + getPointerDescription(pointerArg, *Result.Context) + + "' to " + typeName + + " constructor may cause double deletion"; + } + } + } else if (const auto *resetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset-call")) { + const auto *method = resetCall->getMethodDecl(); + if (method) { + const auto *record = method->getParent(); + if (record) { + std::string typeName = record->getQualifiedNameAsString(); + message = "passing a raw pointer '" + + getPointerDescription(pointerArg, *Result.Context) + + "' to " + typeName + + "::reset() may cause double deletion"; + } + } + } + + if (!message.empty()) { + diag(loc, message); + } + } +} + +std::string SmartPtrInitializationCheck::getPointerDescription( + const Expr *PointerExpr, ASTContext &Context) { + std::string Desc; + llvm::raw_string_ostream OS(Desc); + + // Try to get a readable representation of the expression + PrintingPolicy Policy(Context.getLangOpts()); + Policy.SuppressSpecifiers = false; + Policy.SuppressTagKeyword = true; + + PointerExpr->printPretty(OS, nullptr, Policy); + return OS.str(); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h new file mode 100644 index 00000000000000..b21774b6b08068 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h @@ -0,0 +1,35 @@ +//===----------------------------------------------------------------------===// +// +// 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_SMARTPTRINITIALIZATIONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRINITIALIZATIONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detects dangerous initialization of smart pointers with raw pointers +/// that are already owned elsewhere, which can lead to double deletion. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/smart-ptr-initialization.html +class SmartPtrInitializationCheck : public ClangTidyCheck { +public: + SmartPtrInitializationCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + +private: + std::string getPointerDescription(const Expr *PointerExpr, + ASTContext &Context); +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRINITIALIZATIONCHECK_H \ No newline at end of file diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a6f80e3721db13..76882e0e1a251a 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -202,6 +202,11 @@ New checks Detects default initialization (to 0) of variables with ``enum`` type where the enum has no enumerator with value of 0. +- New :doc:`bugprone-smart-ptr-initialization + <clang-tidy/checks/bugprone/smart-ptr-initialization>` check. + + FIXME: Write a short description. + - New :doc:`cppcoreguidelines-pro-bounds-avoid-unchecked-container-access <clang-tidy/checks/cppcoreguidelines/pro-bounds-avoid-unchecked-container-access>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst new file mode 100644 index 00000000000000..ed72c6b1b92b11 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst @@ -0,0 +1,6 @@ +.. title:: clang-tidy - bugprone-smart-ptr-initialization + +bugprone-smart-ptr-initialization +================================= + +FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 8bb112f3d18329..a6cb12032b4816 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -143,6 +143,7 @@ Clang-Tidy Checks :doc:`bugprone-signed-char-misuse <bugprone/signed-char-misuse>`, :doc:`bugprone-sizeof-container <bugprone/sizeof-container>`, :doc:`bugprone-sizeof-expression <bugprone/sizeof-expression>`, + :doc:`bugprone-smart-ptr-initialization <bugprone/smart-ptr-initialization>`, "Yes" :doc:`bugprone-spuriously-wake-up-functions <bugprone/spuriously-wake-up-functions>`, :doc:`bugprone-standalone-empty <bugprone/standalone-empty>`, "Yes" :doc:`bugprone-std-namespace-modification <bugprone/std-namespace-modification>`, @@ -180,11 +181,12 @@ Clang-Tidy Checks :doc:`bugprone-unused-return-value <bugprone/unused-return-value>`, :doc:`bugprone-use-after-move <bugprone/use-after-move>`, :doc:`bugprone-virtual-near-miss <bugprone/virtual-near-miss>`, "Yes" + :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`, + :doc:`cert-env33-c <cert/env33-c>`, :doc:`cert-err33-c <cert/err33-c>`, - :doc:`cert-err60-cpp <cert/err60-cpp>`, + :doc:`cert-err52-cpp <cert/err52-cpp>`, :doc:`cert-flp30-c <cert/flp30-c>`, - :doc:`cert-msc50-cpp <cert/msc50-cpp>`, - :doc:`cert-oop58-cpp <cert/oop58-cpp>`, + :doc:`cert-mem57-cpp <cert/mem57-cpp>`, :doc:`concurrency-mt-unsafe <concurrency/mt-unsafe>`, :doc:`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous>`, :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines>`, @@ -376,7 +378,7 @@ Clang-Tidy Checks :doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`, :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, "Yes" :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, - :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" + :doc:`readability-braces-around-statements <readability/braces-around-statements>`, :doc:`readability-const-return-type <readability/const-return-type>`, "Yes" :doc:`readability-container-contains <readability/container-contains>`, "Yes" :doc:`readability-container-data-pointer <readability/container-data-pointer>`, "Yes" @@ -445,21 +447,16 @@ Check aliases :doc:`cert-dcl50-cpp <cert/dcl50-cpp>`, :doc:`modernize-avoid-variadic-functions <modernize/avoid-variadic-functions>`, :doc:`cert-dcl51-cpp <cert/dcl51-cpp>`, :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" :doc:`cert-dcl54-cpp <cert/dcl54-cpp>`, :doc:`misc-new-delete-overloads <misc/new-delete-overloads>`, - :doc:`cert-dcl58-cpp <cert/dcl58-cpp>`, :doc:`bugprone-std-namespace-modification <bugprone/std-namespace-modification>`, :doc:`cert-dcl59-cpp <cert/dcl59-cpp>`, :doc:`google-build-namespaces <google/build-namespaces>`, - :doc:`cert-env33-c <cert/env33-c>`, :doc:`bugprone-command-processor <bugprone/command-processor>`, :doc:`cert-err09-cpp <cert/err09-cpp>`, :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`, :doc:`cert-err34-c <cert/err34-c>`, :doc:`bugprone-unchecked-string-to-number-conversion <bugprone/unchecked-string-to-number-conversion>`, - :doc:`cert-err52-cpp <cert/err52-cpp>`, :doc:`modernize-avoid-setjmp-longjmp <modernize/avoid-setjmp-longjmp>`, :doc:`cert-err58-cpp <cert/err58-cpp>`, :doc:`bugprone-throwing-static-initialization <bugprone/throwing-static-initialization>`, :doc:`cert-err60-cpp <cert/err60-cpp>`, :doc:`bugprone-exception-copy-constructor-throws <bugprone/exception-copy-constructor-throws>`, :doc:`cert-err61-cpp <cert/err61-cpp>`, :doc:`misc-throw-by-value-catch-by-reference <misc/throw-by-value-catch-by-reference>`, :doc:`cert-exp42-c <cert/exp42-c>`, :doc:`bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison>`, :doc:`cert-fio38-c <cert/fio38-c>`, :doc:`misc-non-copyable-objects <misc/non-copyable-objects>`, - :doc:`cert-flp30-c <cert/flp30-c>`, :doc:`bugprone-float-loop-counter <bugprone/float-loop-counter>`, :doc:`cert-flp37-c <cert/flp37-c>`, :doc:`bugprone-suspicious-memory-comparison <bugprone/suspicious-memory-comparison>`, :doc:`cert-int09-c <cert/int09-c>`, :doc:`readability-enum-initial-value <readability/enum-initial-value>`, "Yes" - :doc:`cert-mem57-cpp <cert/mem57-cpp>`, :doc:`bugprone-default-operator-new-on-overaligned-type <bugprone/default-operator-new-on-overaligned-type>`, :doc:`cert-msc24-c <cert/msc24-c>`, :doc:`bugprone-unsafe-functions <bugprone/unsafe-functions>`, :doc:`cert-msc30-c <cert/msc30-c>`, :doc:`misc-predictable-rand <misc/predictable-rand>`, :doc:`cert-msc32-c <cert/msc32-c>`, :doc:`bugprone-random-generator-seed <bugprone/random-generator-seed>`, @@ -584,12 +581,12 @@ Check aliases :doc:`cppcoreguidelines-non-private-member-variables-in-classes <cppcoreguidelines/non-private-member-variables-in-classes>`, :doc:`misc-non-private-member-variables-in-classes <misc/non-private-member-variables-in-classes>`, :doc:`cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init>`, :doc:`modernize-use-default-member-init <modernize/use-default-member-init>`, "Yes" :doc:`fuchsia-header-anon-namespaces <fuchsia/header-anon-namespaces>`, :doc:`google-build-namespaces <google/build-namespaces>`, - :doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" + :doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, :doc:`google-readability-function-size <google/readability-function-size>`, :doc:`readability-function-size <readability/function-size>`, :doc:`google-readability-namespace-comments <google/readability-namespace-comments>`, :doc:`llvm-namespace-comment <llvm/namespace-comment>`, :doc:`hicpp-avoid-c-arrays <hicpp/avoid-c-arrays>`, :doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`, :doc:`hicpp-avoid-goto <hicpp/avoid-goto>`, :doc:`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto>`, - :doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" + :doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, :doc:`hicpp-deprecated-headers <hicpp/deprecated-headers>`, :doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes" :doc:`hicpp-explicit-conversions <hicpp/explicit-conversions>`, :doc:`google-explicit-constructor <google/explicit-constructor>`, "Yes" :doc:`hicpp-function-size <hicpp/function-size>`, :doc:`readability-function-size <readability/function-size>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp new file mode 100644 index 00000000000000..ea7c59e25a55ef --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp @@ -0,0 +1,145 @@ +// RUN: %check_clang_tidy %s bugprone-smart-ptr-initialization %t + +namespace std { + +typedef decltype(nullptr) nullptr_t; + +template <typename T> +struct default_delete { + void operator()(T* p) const; +}; + +template <typename T, typename Deleter = default_delete<T>> +class unique_ptr { +public: + unique_ptr(); + explicit unique_ptr(T* p); + unique_ptr(T* p, Deleter d) {} + unique_ptr(std::nullptr_t); + + T* release(); + + void reset(T* p = nullptr); + + template <typename D> + void reset(T* p, D d) {} +}; + +template <typename T> +class shared_ptr { +public: + shared_ptr(); + explicit shared_ptr(T* p); + template <typename Deleter> + shared_ptr(T* p, Deleter d) {} + shared_ptr(std::nullptr_t); + + T* release(); + + void reset(T* p = nullptr); + + template <typename Deleter> + void reset(T* p, Deleter d) {} +}; + +template <typename T> +shared_ptr<T> make_shared(); + +template <typename T> +unique_ptr<T> make_unique(); + +} // namespace std + +struct A { + int x; +}; + +A& getA(); +A* getAPtr(); + +// Should trigger the check for shared_ptr constructor +void test_shared_ptr_constructor() { + std::shared_ptr<A> a(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: passing a raw pointer '&getA()' to std::shared_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger the check for unique_ptr constructor +void test_unique_ptr_constructor() { + std::unique_ptr<A> b(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: passing a raw pointer '&getA()' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger the check for reset() method +void test_reset_method() { + std::shared_ptr<A> sp; + sp.reset(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] + + std::unique_ptr<A> up; + up.reset(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for stack variables +void test_stack_variable() { + int x = 5; + std::unique_ptr<int> ptr(&x); + // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: passing a raw pointer '&x' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for member variables +struct S { + int member; + void test() { + std::unique_ptr<int> ptr(&member); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: passing a raw pointer '&this->member' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] + } +}; + +// Should trigger for pointer returned from function +void test_function_return() { + std::shared_ptr<A> sp(getAPtr()); + // CHECK-MESSAGES: :[[@LINE-1]]:25: warning: passing a raw pointer 'getAPtr()' to std::shared_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should NOT trigger for new expressions - these are OK +void test_new_expression_ok() { + std::shared_ptr<A> a(new A()); + std::unique_ptr<A> b(new A()); +} + +// Should NOT trigger for release() calls - ownership transfer +void test_release_ok() { + auto p1 = std::make_unique<A>(); + std::unique_ptr<A> p2(p1.release()); + + auto p3 = std::make_shared<A>(); + std::shared_ptr<A> p4(p3.release()); +} + +// Should NOT trigger for custom deleters +void test_custom_deleter_ok() { + auto noop_deleter = [](A* p) { }; + std::unique_ptr<A, decltype(noop_deleter)> p1(&getA(), noop_deleter); + std::shared_ptr<A> p2(&getA(), noop_deleter); +} + +// Should NOT trigger for nullptr +void test_nullptr_ok() { + std::shared_ptr<A> a(nullptr); + std::unique_ptr<A> b(nullptr); + std::shared_ptr<A> c; + c.reset(nullptr); +} + +// Should NOT trigger for make_shared/make_unique +void test_make_functions_ok() { + auto sp = std::make_shared<A>(); + auto up = std::make_unique<A>(); +} + +// Edge case: should trigger for array new with wrong smart pointer +void test_array_new() { + std::shared_ptr<A> sp(new A[10]); // This is actually wrong but not our check's concern + // This would be caught by other checks (mismatched new/delete) +} >From 506ab93d2dd5a933bd36c1f0c65d3d77cf9f7b95 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sat, 14 Feb 2026 01:41:16 +0300 Subject: [PATCH 02/14] refactoring && fix nondefault deleter cases --- .../bugprone/SmartPtrInitializationCheck.cpp | 265 +++++++----------- .../smart-ptr-initialization/std_smart_ptr.h | 97 +++++++ .../smart-ptr-initialization-array.cpp | 1 + .../bugprone/smart-ptr-initialization.cpp | 83 +++--- 4 files changed, 233 insertions(+), 213 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp index 392c193918be04..d8e557668c6f69 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -8,210 +8,151 @@ #include "SmartPtrInitializationCheck.h" #include "clang/AST/ASTContext.h" +#include "clang/AST/DeclCXX.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/ASTMatchers/ASTMatchers.h" -#include "clang/Basic/Diagnostic.h" -#include "clang/Lex/Lexer.h" -using namespace clang; using namespace clang::ast_matchers; namespace clang::tidy::bugprone { namespace { -AST_MATCHER(Expr, isNewExpression) { - return isa<CXXNewExpr>(Node); -} - -AST_MATCHER(Expr, isReleaseCall) { - const auto *call = dyn_cast<CallExpr>(&Node); - if (!call) - return false; - - const auto *method = dyn_cast<CXXMemberCallExpr>(call); - if (!method) +// Helper function to check if a smart pointer record type has a custom deleter +// based on the record type and number of arguments in the call/constructor +static bool hasCustomDeleterForRecord(const CXXRecordDecl *Record, + unsigned NumArgs) { + if (!Record) return false; - const auto *member = method->getMethodDecl(); - if (!member) - return false; - - return member->getName() == "release"; -} - -AST_MATCHER(Expr, isMakeUniqueOrSharedCall) { - const auto *call = dyn_cast<CallExpr>(&Node); - if (!call) - return false; - - const auto *callee = call->getDirectCallee(); - if (!callee) - return false; - - StringRef name = callee->getName(); - if (name != "make_unique" && name != "make_shared") - return false; - - // Check if it's in std namespace by checking the qualified name - std::string qualifiedName = callee->getQualifiedNameAsString(); - return qualifiedName == "std::make_unique" || qualifiedName == "std::make_shared"; -} - -AST_MATCHER(CXXConstructExpr, hasCustomDeleter) { - if (Node.getNumArgs() < 2) - return false; - - // Check if the second argument is a deleter - const Expr *deleterArg = Node.getArg(1); - if (!deleterArg) - return false; - - // Check if this is a smart pointer construction with custom deleter - const auto *record = Node.getConstructor()->getParent(); - if (!record) - return false; + const std::string typeName = Record->getQualifiedNameAsString(); + if (typeName == "std::shared_ptr") { + // Check if the second argument is a deleter + if (NumArgs >= 2) + return true; + } else if (typeName == "std::unique_ptr") { + // Check if the second template argument is a deleter + const auto *templateSpec = + dyn_cast<ClassTemplateSpecializationDecl>(Record); + if (!templateSpec) + return false; + + const auto &templateArgs = templateSpec->getTemplateArgs(); + // unique_ptr has at least 1 template argument (the pointer type) + // If it has 2, the second one is the deleter type + if (templateArgs.size() >= 2) { + const auto &deleterArg = templateArgs[1]; + // The deleter must be a type + if (deleterArg.getKind() == TemplateArgument::Type) { + QualType deleterType = deleterArg.getAsType(); + if (auto *deleterRecord = deleterType->getAsCXXRecordDecl()) { + const std::string DeleterTypeName = + deleterRecord->getQualifiedNameAsString(); + if (DeleterTypeName != "std::default_delete") + return true; + } + } + } + } - std::string typeName = record->getQualifiedNameAsString(); - return typeName == "std::shared_ptr" || typeName == "std::unique_ptr"; + return false; } -AST_MATCHER(CallExpr, isResetWithCustomDeleter) { - const auto *memberCall = dyn_cast<CXXMemberCallExpr>(&Node); - if (!memberCall) - return false; +// TODO: all types must be in config +// TODO: boost::shared_ptr and boost::unique_ptr +// TODO: reset and release must be in config +AST_MATCHER(Stmt, hasCustomDeleter) { + const auto *constructExpr = dyn_cast<CXXConstructExpr>(&Node); + if (constructExpr) { + const auto *record = constructExpr->getConstructor()->getParent(); + return hasCustomDeleterForRecord(record, constructExpr->getNumArgs()); + } - const auto *method = memberCall->getMethodDecl(); - if (!method || method->getName() != "reset") - return false; + const auto *callExpr = dyn_cast<CallExpr>(&Node); + if (callExpr) { + const auto *memberCall = dyn_cast<CXXMemberCallExpr>(callExpr); + if (!memberCall) + return false; - if (Node.getNumArgs() < 2) - return false; + const auto *method = memberCall->getMethodDecl(); + if (!method || method->getName() != "reset") + return false; - const auto *record = method->getParent(); - if (!record) - return false; + const auto *record = method->getParent(); + return hasCustomDeleterForRecord(record, callExpr->getNumArgs()); + } - std::string typeName = record->getQualifiedNameAsString(); - return typeName == "std::shared_ptr" || typeName == "std::unique_ptr"; + return false; } } // namespace void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { + auto ReleaseCallMatcher = + cxxMemberCallExpr(callee(cxxMethodDecl(hasName("release")))); // Matcher for smart pointer constructors - auto smartPtrConstructorMatcher = cxxConstructExpr( - hasDeclaration( - cxxConstructorDecl(ofClass(hasAnyName("std::shared_ptr", - "std::unique_ptr")))), - hasArgument(0, expr().bind("pointer-arg")), - unless(hasCustomDeleter()), - unless(hasArgument(0, isNewExpression())), - unless(hasArgument(0, isReleaseCall())), - unless(hasArgument(0, isMakeUniqueOrSharedCall())) - ).bind("constructor"); + auto smartPtrConstructorMatcher = + cxxConstructExpr( + hasDeclaration(cxxConstructorDecl( + ofClass(hasAnyName("std::shared_ptr", "std::unique_ptr")), + unless(anyOf(isCopyConstructor(), isMoveConstructor())))), + hasArgument(0, expr(unless(nullPointerConstant())).bind("pointer-arg")), + unless(hasCustomDeleter()), unless(hasArgument(0, cxxNewExpr())), + unless(hasArgument(0, ReleaseCallMatcher))) + .bind("constructor"); // Matcher for reset() calls - auto resetCallMatcher = cxxMemberCallExpr( - on(hasType(cxxRecordDecl(hasAnyName("std::shared_ptr", "std::unique_ptr")))), - callee(cxxMethodDecl(hasName("reset"))), - hasArgument(0, expr().bind("pointer-arg")), - unless(isResetWithCustomDeleter()), - unless(hasArgument(0, isNewExpression())), - unless(hasArgument(0, isReleaseCall())), - unless(hasArgument(0, isMakeUniqueOrSharedCall())) - ).bind("reset-call"); + auto resetCallMatcher = + cxxMemberCallExpr(on(hasType(cxxRecordDecl( + hasAnyName("std::shared_ptr", "std::unique_ptr")))), + callee(cxxMethodDecl(hasName("reset"))), + hasArgument(0, expr(unless(nullPointerConstant())).bind("pointer-arg")), + unless(hasCustomDeleter()), + unless(hasArgument(0, cxxNewExpr())), + unless(hasArgument(0, ReleaseCallMatcher))) + .bind("reset-call"); Finder->addMatcher(smartPtrConstructorMatcher, this); Finder->addMatcher(resetCallMatcher, this); } -void SmartPtrInitializationCheck::check(const MatchFinder::MatchResult &Result) { +void SmartPtrInitializationCheck::check( + const MatchFinder::MatchResult &Result) { const auto *pointerArg = Result.Nodes.getNodeAs<Expr>("pointer-arg"); - if (!pointerArg) - return; - - // Skip if the pointer is a null pointer - if (pointerArg->isNullPointerConstant(*Result.Context, - Expr::NPC_ValueDependentIsNotNull)) - return; - - // Check if the expression is a call to a function returning a pointer - bool isFunctionReturn = false; - if (const auto *call = dyn_cast<CallExpr>(pointerArg)) { - if (call->getDirectCallee()) { - isFunctionReturn = true; - } - } - - // Check if it's taking address of something - bool isAddressOf = isa<UnaryOperator>(pointerArg) && - cast<UnaryOperator>(pointerArg)->getOpcode() == UO_AddrOf; - - // Check if it's getting pointer from reference - const Expr *innerExpr = pointerArg->IgnoreParenCasts(); - if (const auto *unaryOp = dyn_cast<UnaryOperator>(innerExpr)) { - if (unaryOp->getOpcode() == UO_AddrOf) { - isAddressOf = true; - } - } - - // Also check for member expressions that might return references - if (const auto *memberExpr = dyn_cast<MemberExpr>(innerExpr)) { - if (memberExpr->isArrow()) { - // arrow operator returns pointer, not reference - isAddressOf = false; - } - } - - if (isFunctionReturn || isAddressOf) { - std::string message; - const SourceLocation loc = pointerArg->getBeginLoc(); - - if (const auto *constructor = - Result.Nodes.getNodeAs<CXXConstructExpr>("constructor")) { - const auto *decl = constructor->getConstructor(); - if (decl) { - const auto *record = decl->getParent(); - if (record) { - std::string typeName = record->getQualifiedNameAsString(); - message = "passing a raw pointer '" + - getPointerDescription(pointerArg, *Result.Context) + - "' to " + typeName + - " constructor may cause double deletion"; - } - } - } else if (const auto *resetCall = - Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset-call")) { - const auto *method = resetCall->getMethodDecl(); - if (method) { - const auto *record = method->getParent(); - if (record) { - std::string typeName = record->getQualifiedNameAsString(); - message = "passing a raw pointer '" + - getPointerDescription(pointerArg, *Result.Context) + - "' to " + typeName + - "::reset() may cause double deletion"; - } - } - } - - if (!message.empty()) { - diag(loc, message); + const auto *constructor = + Result.Nodes.getNodeAs<CXXConstructExpr>("constructor"); + const auto *ResetCall = + Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset-call"); + assert(pointerArg); + + const SourceLocation loc = pointerArg->getBeginLoc(); + const CXXMethodDecl *MD = + constructor ? constructor->getConstructor() + : (ResetCall ? ResetCall->getMethodDecl() : nullptr); + + if (MD) { + const auto *record = MD->getParent(); + if (record) { + const std::string typeName = record->getQualifiedNameAsString(); + diag(loc, + "passing a raw pointer '%0' to %1%2 may cause double deletion") + << getPointerDescription(pointerArg, *Result.Context) << typeName + << (constructor ? " constructor" : "::reset()"); } } } -std::string SmartPtrInitializationCheck::getPointerDescription( - const Expr *PointerExpr, ASTContext &Context) { +std::string +SmartPtrInitializationCheck::getPointerDescription(const Expr *PointerExpr, + ASTContext &Context) { std::string Desc; llvm::raw_string_ostream OS(Desc); - + // Try to get a readable representation of the expression PrintingPolicy Policy(Context.getLangOpts()); Policy.SuppressSpecifiers = false; Policy.SuppressTagKeyword = true; - + PointerExpr->printPretty(OS, nullptr, Policy); return OS.str(); } diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h new file mode 100644 index 00000000000000..380ee126738a2e --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h @@ -0,0 +1,97 @@ +namespace std { + +typedef decltype(nullptr) nullptr_t; +typedef unsigned long size_t; + +template <typename T> +struct default_delete { + void operator()(T* p) const; +}; + +template <typename T> +struct default_delete<T[]> { + void operator()(T* p) const; +}; + +template <typename T, typename Deleter = default_delete<T>> +class unique_ptr { +public: + unique_ptr(); + explicit unique_ptr(T* p); + unique_ptr(T* p, Deleter d) {} + unique_ptr(std::nullptr_t); + + T* release(); + + void reset(T* p = nullptr); + + template <typename D> + void reset(T* p, D d) {} +}; + +template <typename T, typename Deleter> +class unique_ptr<T[], Deleter> { +public: + unique_ptr(); + template <typename U> + explicit unique_ptr(U* p); + template <typename U> + unique_ptr(U* p, Deleter d) {} + unique_ptr(std::nullptr_t); + + T* release(); + + void reset(T* p = nullptr); + + template <typename D> + void reset(T* p, D d) {} +}; + +template <typename T> +class shared_ptr { +public: + shared_ptr(); + explicit shared_ptr(T* p); + template <typename Deleter> + shared_ptr(T* p, Deleter d) {} + shared_ptr(std::nullptr_t); + + T* release(); + + void reset(T* p = nullptr); + + template <typename Deleter> + void reset(T* p, Deleter d) {} +}; + +template <typename T> +class shared_ptr<T[]> { +public: + shared_ptr(); + template <typename U> + explicit shared_ptr(U* p); + template <typename U, typename Deleter> + shared_ptr(U* p, Deleter d) {} + shared_ptr(std::nullptr_t); + + T* release(); + + void reset(T* p = nullptr); + + template <typename Deleter> + void reset(T* p, Deleter d) {} +}; + +template <typename T> +shared_ptr<T> make_shared(); + +template <typename T> +shared_ptr<T[]> make_shared(std::size_t n); + +template <typename T> +unique_ptr<T> make_unique(); + +template <typename T> +unique_ptr<T[]> make_unique(std::size_t n); + +} // namespace std diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp new file mode 100644 index 00000000000000..aade2f7a10c6f8 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp @@ -0,0 +1 @@ +// RUN: %check_clang_tidy %s bugprone-smart-ptr-initialization %t -- -- -I%S diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp index ea7c59e25a55ef..a9cf0318c97def 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp @@ -1,54 +1,6 @@ -// RUN: %check_clang_tidy %s bugprone-smart-ptr-initialization %t +// RUN: %check_clang_tidy %s bugprone-smart-ptr-initialization %t -- -- -I%S -namespace std { - -typedef decltype(nullptr) nullptr_t; - -template <typename T> -struct default_delete { - void operator()(T* p) const; -}; - -template <typename T, typename Deleter = default_delete<T>> -class unique_ptr { -public: - unique_ptr(); - explicit unique_ptr(T* p); - unique_ptr(T* p, Deleter d) {} - unique_ptr(std::nullptr_t); - - T* release(); - - void reset(T* p = nullptr); - - template <typename D> - void reset(T* p, D d) {} -}; - -template <typename T> -class shared_ptr { -public: - shared_ptr(); - explicit shared_ptr(T* p); - template <typename Deleter> - shared_ptr(T* p, Deleter d) {} - shared_ptr(std::nullptr_t); - - T* release(); - - void reset(T* p = nullptr); - - template <typename Deleter> - void reset(T* p, Deleter d) {} -}; - -template <typename T> -shared_ptr<T> make_shared(); - -template <typename T> -unique_ptr<T> make_unique(); - -} // namespace std +#include "Inputs/smart-ptr-initialization/std_smart_ptr.h" struct A { int x; @@ -57,6 +9,8 @@ struct A { A& getA(); A* getAPtr(); +// TODO: std::shared_ptr<A[]> also must be supported + // Should trigger the check for shared_ptr constructor void test_shared_ptr_constructor() { std::shared_ptr<A> a(&getA()); @@ -69,17 +23,34 @@ void test_unique_ptr_constructor() { // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: passing a raw pointer '&getA()' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] } +// TODO: all `reset` tests must be separately + // Should trigger the check for reset() method void test_reset_method() { std::shared_ptr<A> sp; sp.reset(&getA()); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] - + std::unique_ptr<A> up; up.reset(&getA()); // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] } +// Should NOT trigger the check for reset() method with custom deleter +void test_reset_method_with_custom_deleter() { + auto noop_deleter = [](A* p) { }; + std::shared_ptr<A> sp(nullptr, noop_deleter); + std::unique_ptr<A, decltype(noop_deleter)> up(nullptr, noop_deleter); + + sp.reset(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] + // doesn't have deleter anymore + + sp.reset(&getA(), noop_deleter); + + up.reset(&getA()); +} + // Should trigger for stack variables void test_stack_variable() { int x = 5; @@ -104,6 +75,7 @@ void test_function_return() { // Should NOT trigger for new expressions - these are OK void test_new_expression_ok() { + // TODO: forbid to pass `new A[];`?? std::shared_ptr<A> a(new A()); std::unique_ptr<A> b(new A()); } @@ -117,9 +89,14 @@ void test_release_ok() { std::shared_ptr<A> p4(p3.release()); } +struct NoopDeleter { + void operator() (A* p) {} +}; + // Should NOT trigger for custom deleters void test_custom_deleter_ok() { auto noop_deleter = [](A* p) { }; + std::unique_ptr<A, NoopDeleter> p0(&getA()); std::unique_ptr<A, decltype(noop_deleter)> p1(&getA(), noop_deleter); std::shared_ptr<A> p2(&getA(), noop_deleter); } @@ -138,6 +115,10 @@ void test_make_functions_ok() { auto up = std::make_unique<A>(); } +// TODO: write that this is a job for bugprone-shared-ptr-array-mismatch +// TODO: the same test, but for smart pointer of array +// TODO: the same test, but with `release` call +// // Edge case: should trigger for array new with wrong smart pointer void test_array_new() { std::shared_ptr<A> sp(new A[10]); // This is actually wrong but not our check's concern >From ab4495950b221d02fb77e7e2a0f7542b25ec9c75 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 01:33:13 +0300 Subject: [PATCH 03/14] refactoring --- .../bugprone/SmartPtrInitializationCheck.cpp | 127 +++++++----------- 1 file changed, 45 insertions(+), 82 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp index d8e557668c6f69..e925b382c277f2 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -18,98 +18,61 @@ namespace clang::tidy::bugprone { namespace { -// Helper function to check if a smart pointer record type has a custom deleter -// based on the record type and number of arguments in the call/constructor -static bool hasCustomDeleterForRecord(const CXXRecordDecl *Record, - unsigned NumArgs) { - if (!Record) - return false; - - const std::string typeName = Record->getQualifiedNameAsString(); - if (typeName == "std::shared_ptr") { - // Check if the second argument is a deleter - if (NumArgs >= 2) - return true; - } else if (typeName == "std::unique_ptr") { - // Check if the second template argument is a deleter - const auto *templateSpec = - dyn_cast<ClassTemplateSpecializationDecl>(Record); - if (!templateSpec) - return false; - - const auto &templateArgs = templateSpec->getTemplateArgs(); - // unique_ptr has at least 1 template argument (the pointer type) - // If it has 2, the second one is the deleter type - if (templateArgs.size() >= 2) { - const auto &deleterArg = templateArgs[1]; - // The deleter must be a type - if (deleterArg.getKind() == TemplateArgument::Type) { - QualType deleterType = deleterArg.getAsType(); - if (auto *deleterRecord = deleterType->getAsCXXRecordDecl()) { - const std::string DeleterTypeName = - deleterRecord->getQualifiedNameAsString(); - if (DeleterTypeName != "std::default_delete") - return true; - } - } - } - } - - return false; -} - // TODO: all types must be in config // TODO: boost::shared_ptr and boost::unique_ptr // TODO: reset and release must be in config -AST_MATCHER(Stmt, hasCustomDeleter) { - const auto *constructExpr = dyn_cast<CXXConstructExpr>(&Node); - if (constructExpr) { - const auto *record = constructExpr->getConstructor()->getParent(); - return hasCustomDeleterForRecord(record, constructExpr->getNumArgs()); - } - - const auto *callExpr = dyn_cast<CallExpr>(&Node); - if (callExpr) { - const auto *memberCall = dyn_cast<CXXMemberCallExpr>(callExpr); - if (!memberCall) - return false; - - const auto *method = memberCall->getMethodDecl(); - if (!method || method->getName() != "reset") - return false; - - const auto *record = method->getParent(); - return hasCustomDeleterForRecord(record, callExpr->getNumArgs()); - } - - return false; -} } // namespace void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { auto ReleaseCallMatcher = cxxMemberCallExpr(callee(cxxMethodDecl(hasName("release")))); + + auto UniquePtrWithCustomDeleter = classTemplateSpecializationDecl( + hasName("std::unique_ptr"), templateArgumentCountIs(2), + hasTemplateArgument(1, refersToType(unless(hasDeclaration(cxxRecordDecl( + hasName("std::default_delete"))))))); + // Matcher for smart pointer constructors + // Exclude constructors with custom deleters: + // - shared_ptr with 2+ arguments (second is deleter) + // - unique_ptr with 2+ template args where second is not default_delete + auto HasCustomDeleter = anyOf( + allOf(hasDeclaration( + cxxConstructorDecl(ofClass(hasName("std::shared_ptr")))), + hasArgument(1, anything())), + hasDeclaration(cxxConstructorDecl(ofClass(UniquePtrWithCustomDeleter)))); + auto smartPtrConstructorMatcher = cxxConstructExpr( hasDeclaration(cxxConstructorDecl( ofClass(hasAnyName("std::shared_ptr", "std::unique_ptr")), unless(anyOf(isCopyConstructor(), isMoveConstructor())))), - hasArgument(0, expr(unless(nullPointerConstant())).bind("pointer-arg")), - unless(hasCustomDeleter()), unless(hasArgument(0, cxxNewExpr())), + hasArgument(0, + expr(unless(nullPointerConstant())).bind("pointer-arg")), + unless(HasCustomDeleter), unless(hasArgument(0, cxxNewExpr())), unless(hasArgument(0, ReleaseCallMatcher))) .bind("constructor"); // Matcher for reset() calls + // Exclude reset() calls with custom deleters: + // - shared_ptr with 2+ arguments (second is deleter) + // - unique_ptr with custom deleter type (2+ template args where second is not + // default_delete) + auto HasCustomDeleterInReset = + anyOf(allOf(on(hasType(cxxRecordDecl(hasName("std::shared_ptr")))), + hasArgument(1, anything())), + on(hasType(qualType(hasDeclaration(UniquePtrWithCustomDeleter))))); + auto resetCallMatcher = - cxxMemberCallExpr(on(hasType(cxxRecordDecl( - hasAnyName("std::shared_ptr", "std::unique_ptr")))), - callee(cxxMethodDecl(hasName("reset"))), - hasArgument(0, expr(unless(nullPointerConstant())).bind("pointer-arg")), - unless(hasCustomDeleter()), - unless(hasArgument(0, cxxNewExpr())), - unless(hasArgument(0, ReleaseCallMatcher))) + cxxMemberCallExpr( + on(hasType( + cxxRecordDecl(hasAnyName("std::shared_ptr", "std::unique_ptr")))), + callee(cxxMethodDecl(hasName("reset"))), + hasArgument(0, + expr(unless(nullPointerConstant())).bind("pointer-arg")), + unless(HasCustomDeleterInReset), unless(hasArgument(0, cxxNewExpr())), + unless(hasArgument(0, ReleaseCallMatcher))) .bind("reset-call"); Finder->addMatcher(smartPtrConstructorMatcher, this); @@ -129,17 +92,17 @@ void SmartPtrInitializationCheck::check( const CXXMethodDecl *MD = constructor ? constructor->getConstructor() : (ResetCall ? ResetCall->getMethodDecl() : nullptr); + if (!MD) + return; + + const auto *record = MD->getParent(); + if (!record) + return; - if (MD) { - const auto *record = MD->getParent(); - if (record) { - const std::string typeName = record->getQualifiedNameAsString(); - diag(loc, - "passing a raw pointer '%0' to %1%2 may cause double deletion") - << getPointerDescription(pointerArg, *Result.Context) << typeName - << (constructor ? " constructor" : "::reset()"); - } - } + const std::string typeName = record->getQualifiedNameAsString(); + diag(loc, "passing a raw pointer '%0' to %1%2 may cause double deletion") + << getPointerDescription(pointerArg, *Result.Context) << typeName + << (constructor ? " constructor" : "::reset()"); } std::string >From 1bf34c325e12c5940aabaa5d3685301f4c566a67 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 16:39:10 +0300 Subject: [PATCH 04/14] Improve tests --- .../smart-ptr-initialization/std_smart_ptr.h | 20 +-- .../smart-ptr-initialization-array-cxx17.cpp | 116 ++++++++++++++++ .../smart-ptr-initialization-array.cpp | 119 ++++++++++++++++- .../bugprone/smart-ptr-initialization.cpp | 126 +++++++++++------- 4 files changed, 326 insertions(+), 55 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array-cxx17.cpp diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h index 380ee126738a2e..afb9a2e95792a2 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/Inputs/smart-ptr-initialization/std_smart_ptr.h @@ -82,16 +82,20 @@ class shared_ptr<T[]> { void reset(T* p, Deleter d) {} }; -template <typename T> -shared_ptr<T> make_shared(); +template<typename T> + struct remove_reference + { using type = T; }; -template <typename T> -shared_ptr<T[]> make_shared(std::size_t n); +template<typename T> + struct remove_reference<T&> + { using type = T; }; -template <typename T> -unique_ptr<T> make_unique(); +template<typename T> + struct remove_reference<T&&> + { using type = T; }; -template <typename T> -unique_ptr<T[]> make_unique(std::size_t n); +template<typename T> + constexpr typename std::remove_reference<T>::type&& + move(T&& t) noexcept; } // namespace std diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array-cxx17.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array-cxx17.cpp new file mode 100644 index 00000000000000..f679c67188d4ea --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array-cxx17.cpp @@ -0,0 +1,116 @@ +// RUN: %check_clang_tidy -std=c++17-or-later %s bugprone-smart-ptr-initialization %t -- -- -I%S + +#include "Inputs/smart-ptr-initialization/std_smart_ptr.h" + +struct A { + int x; +}; + +A arr[10]; + +// Should trigger the check for shared_ptr constructor +void test_shared_ptr_constructor() { + std::shared_ptr<A[]> a(arr); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: passing a raw pointer 'arr' to std::shared_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for stack variables +void test_stack_variable() { + int x[10] = {5}; + std::shared_ptr<int[]> ptr(x); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: passing a raw pointer 'x' to std::shared_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for member variables +struct S { + int member[10]; + void test() { + std::shared_ptr<int[]> ptr(member); + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: passing a raw pointer 'this->member' to std::shared_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] + } +}; + +// Should NOT trigger for new expressions - these are OK +void test_new_expression_ok() { + std::shared_ptr<A[]> a(new A[10]); +} + +// Should NOT trigger for release() calls - ownership transfer +void test_release_ok(std::shared_ptr<A[]> p3) { + std::shared_ptr<A[]> p4(p3.release()); +} + +struct NoopDeleter { + void operator() (A* p) {} +}; + +// Should NOT trigger for custom deleters +void test_custom_deleter_ok() { + auto noop_deleter = [](A* p) { }; + std::shared_ptr<A[]> p2(arr, noop_deleter); +} + +// Should NOT trigger for nullptr +void test_nullptr_ok() { + std::shared_ptr<A[]> a(nullptr); +} + +// Should NOT trigger for copy and move constructors +void test_copy_move_constructor_ok(std::shared_ptr<A[]> sp) { + auto sp2 = sp; + auto sp3 = std::move(sp); +} + +// Should trigger the check for shared_ptr reset +void test_shared_ptr_reset() { + std::shared_ptr<A[]> a; + a.reset(arr); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: passing a raw pointer 'arr' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for stack variables with reset +void test_stack_variable_reset() { + int x[10] = {5}; + std::shared_ptr<int[]> ptr; + ptr.reset(x); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: passing a raw pointer 'x' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should NOT trigger for new expressions with reset - these are OK +void test_new_expression_reset_ok() { + std::shared_ptr<A[]> a; + a.reset(new A[10]); +} + +// Should NOT trigger for release() calls with reset - ownership transfer +void test_release_reset_ok(std::shared_ptr<A[]> p3) { + std::shared_ptr<A[]> p4; + p4.reset(p3.release()); +} + +// Should NOT trigger for custom deleters with reset +void test_custom_deleter_reset_ok() { + auto noop_deleter = [](A* p) { }; + std::shared_ptr<A[]> p2; + p2.reset(arr, noop_deleter); +} + +// Should NOT trigger for nullptr with reset +void test_nullptr_reset_ok() { + std::shared_ptr<A[]> a; + a.reset(nullptr); +} + +// +// Edge case: should trigger for array new with wrong smart pointer +void test_array_new() { + std::shared_ptr<A[]> sp(new A); // This is actually wrong but not our check's concern + sp.reset(new A); + // This would be caught by bugprone-shared-ptr-array-mismatch checks +} + +void test_array_release(std::shared_ptr<A> spa) { + std::shared_ptr<A[]> sp(spa.release()); // This is actually wrong but not our check's concern + sp.reset(spa.release()); + // This would be caught by bugprone-shared-ptr-array-mismatch checks (mismatched new/delete) +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp index aade2f7a10c6f8..807d34f126b296 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization-array.cpp @@ -1 +1,118 @@ -// RUN: %check_clang_tidy %s bugprone-smart-ptr-initialization %t -- -- -I%S +// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-smart-ptr-initialization %t -- -- -I%S + +#include "Inputs/smart-ptr-initialization/std_smart_ptr.h" + +struct A { + int x; +}; + +A arr[10]; + +// Should trigger the check for unique_ptr constructor +void test_unique_ptr_constructor() { + std::unique_ptr<A[]> b(arr); + // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: passing a raw pointer 'arr' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for stack variables +void test_stack_variable() { + int x[10] = {5}; + std::unique_ptr<int[]> ptr(x); + // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: passing a raw pointer 'x' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for member variables +struct S { + int member[10]; + void test() { + std::unique_ptr<int[]> ptr(member); + // CHECK-MESSAGES: :[[@LINE-1]]:32: warning: passing a raw pointer 'this->member' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] + } +}; + +// Should NOT trigger for new expressions - these are OK +void test_new_expression_ok() { + std::unique_ptr<A[]> b(new A[10]); +} + +// Should NOT trigger for release() calls - ownership transfer +void test_release_ok(std::unique_ptr<A[]> p1) { + std::unique_ptr<A[]> p2(p1.release()); +} + +struct NoopDeleter { + void operator() (A* p) {} +}; + +// Should NOT trigger for custom deleters +void test_custom_deleter_ok() { + auto noop_deleter = [](A* p) { }; + std::unique_ptr<A[], NoopDeleter> p0(arr); + std::unique_ptr<A[], decltype(noop_deleter)> p1(arr, noop_deleter); +} + +// Should NOT trigger for nullptr +void test_nullptr_ok() { + std::unique_ptr<A[]> b(nullptr); +} + +// Should NOT trigger for copy and move constructors +void test_copy_move_constructor_ok(std::unique_ptr<A[]> up) { + auto up3 = std::move(up); +} + +// Should trigger the check for unique_ptr reset +void test_unique_ptr_reset() { + std::unique_ptr<A[]> b; + b.reset(arr); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: passing a raw pointer 'arr' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for stack variables with reset +void test_stack_variable_reset() { + int x[10] = {5}; + std::unique_ptr<int[]> ptr; + ptr.reset(x); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: passing a raw pointer 'x' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should NOT trigger for new expressions with reset - these are OK +void test_new_expression_reset_ok() { + std::unique_ptr<A[]> b; + b.reset(new A[10]); +} + +// Should NOT trigger for release() calls with reset - ownership transfer +void test_release_reset_ok(std::unique_ptr<A[]> p1) { + std::unique_ptr<A[]> p2; + p2.reset(p1.release()); +} + +// Should NOT trigger for custom deleters with reset +void test_custom_deleter_reset_ok() { + auto noop_deleter = [](A* p) { }; + std::unique_ptr<A[], NoopDeleter> p0; + p0.reset(arr); + std::unique_ptr<A[], decltype(noop_deleter)> p1; + p1.reset(arr, noop_deleter); +} + +// Should NOT trigger for nullptr with reset +void test_nullptr_reset_ok() { + std::unique_ptr<A[]> b; + b.reset(nullptr); +} + +// +// Edge case: should trigger for array new with wrong smart pointer +void test_array_new() { + std::unique_ptr<A[]> sp(new A); // This is actually wrong but not our check's concern + sp.reset(new A); + // This would be caught by bugprone-shared-ptr-array-mismatch checks +} + +void test_array_release(std::unique_ptr<A> spa) { + std::unique_ptr<A[]> sp(spa.release()); // This is actually wrong but not our check's concern + sp.reset(spa.release()); + // This would be caught by bugprone-shared-ptr-array-mismatch checks (mismatched new/delete) +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp index a9cf0318c97def..a9779c55b26f6e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/smart-ptr-initialization.cpp @@ -1,4 +1,4 @@ -// RUN: %check_clang_tidy %s bugprone-smart-ptr-initialization %t -- -- -I%S +// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-smart-ptr-initialization %t -- -- -I%S #include "Inputs/smart-ptr-initialization/std_smart_ptr.h" @@ -9,8 +9,6 @@ struct A { A& getA(); A* getAPtr(); -// TODO: std::shared_ptr<A[]> also must be supported - // Should trigger the check for shared_ptr constructor void test_shared_ptr_constructor() { std::shared_ptr<A> a(&getA()); @@ -23,34 +21,6 @@ void test_unique_ptr_constructor() { // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: passing a raw pointer '&getA()' to std::unique_ptr constructor may cause double deletion [bugprone-smart-ptr-initialization] } -// TODO: all `reset` tests must be separately - -// Should trigger the check for reset() method -void test_reset_method() { - std::shared_ptr<A> sp; - sp.reset(&getA()); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] - - std::unique_ptr<A> up; - up.reset(&getA()); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] -} - -// Should NOT trigger the check for reset() method with custom deleter -void test_reset_method_with_custom_deleter() { - auto noop_deleter = [](A* p) { }; - std::shared_ptr<A> sp(nullptr, noop_deleter); - std::unique_ptr<A, decltype(noop_deleter)> up(nullptr, noop_deleter); - - sp.reset(&getA()); - // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer '&getA()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] - // doesn't have deleter anymore - - sp.reset(&getA(), noop_deleter); - - up.reset(&getA()); -} - // Should trigger for stack variables void test_stack_variable() { int x = 5; @@ -75,17 +45,13 @@ void test_function_return() { // Should NOT trigger for new expressions - these are OK void test_new_expression_ok() { - // TODO: forbid to pass `new A[];`?? std::shared_ptr<A> a(new A()); std::unique_ptr<A> b(new A()); } // Should NOT trigger for release() calls - ownership transfer -void test_release_ok() { - auto p1 = std::make_unique<A>(); +void test_release_ok(std::unique_ptr<A> p1, std::shared_ptr<A> p3) { std::unique_ptr<A> p2(p1.release()); - - auto p3 = std::make_shared<A>(); std::shared_ptr<A> p4(p3.release()); } @@ -105,22 +71,90 @@ void test_custom_deleter_ok() { void test_nullptr_ok() { std::shared_ptr<A> a(nullptr); std::unique_ptr<A> b(nullptr); - std::shared_ptr<A> c; - c.reset(nullptr); } -// Should NOT trigger for make_shared/make_unique -void test_make_functions_ok() { - auto sp = std::make_shared<A>(); - auto up = std::make_unique<A>(); +// Should NOT trigger for copy and move constructors +void test_copy_move_constructor_ok(std::shared_ptr<A> sp, std::unique_ptr<A> up) { + auto sp2 = sp; + + auto sp3 = std::move(sp); + auto up3 = std::move(up); +} + +// Should trigger the check for shared_ptr reset +void test_shared_ptr_reset() { + std::shared_ptr<A> a; + a.reset(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: passing a raw pointer '&getA()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger the check for unique_ptr reset +void test_unique_ptr_reset() { + std::unique_ptr<A> b; + b.reset(&getA()); + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: passing a raw pointer '&getA()' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for stack variables with reset +void test_stack_variable_reset() { + int x = 5; + std::unique_ptr<int> ptr; + ptr.reset(&x); + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: passing a raw pointer '&x' to std::unique_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should trigger for pointer returned from function with reset +void test_function_return_reset() { + std::shared_ptr<A> sp; + sp.reset(getAPtr()); + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: passing a raw pointer 'getAPtr()' to std::shared_ptr::reset() may cause double deletion [bugprone-smart-ptr-initialization] +} + +// Should NOT trigger for new expressions with reset - these are OK +void test_new_expression_reset_ok() { + std::shared_ptr<A> a; + a.reset(new A()); + std::unique_ptr<A> b; + b.reset(new A()); +} + +// Should NOT trigger for release() calls with reset - ownership transfer +void test_release_reset_ok(std::unique_ptr<A> p1, std::shared_ptr<A> p3) { + std::unique_ptr<A> p2; + p2.reset(p1.release()); + std::shared_ptr<A> p4; + p4.reset(p3.release()); +} + +// Should NOT trigger for custom deleters with reset +void test_custom_deleter_reset_ok() { + auto noop_deleter = [](A* p) { }; + std::unique_ptr<A, NoopDeleter> p0; + p0.reset(&getA()); + std::unique_ptr<A, decltype(noop_deleter)> p1; + p1.reset(&getA(), noop_deleter); + std::shared_ptr<A> p2; + p2.reset(&getA(), noop_deleter); +} + +// Should NOT trigger for nullptr with reset +void test_nullptr_reset_ok() { + std::shared_ptr<A> a; + a.reset(nullptr); + std::unique_ptr<A> b; + b.reset(nullptr); } -// TODO: write that this is a job for bugprone-shared-ptr-array-mismatch -// TODO: the same test, but for smart pointer of array -// TODO: the same test, but with `release` call // // Edge case: should trigger for array new with wrong smart pointer void test_array_new() { std::shared_ptr<A> sp(new A[10]); // This is actually wrong but not our check's concern - // This would be caught by other checks (mismatched new/delete) + sp.reset(new A[10]); + // This would be caught by bugprone-shared-ptr-array-mismatch checks +} + +void test_array_release(std::shared_ptr<A[]> spa) { + std::shared_ptr<A> sp(spa.release()); // This is actually wrong but not our check's concern + sp.reset(spa.release()); + // This would be caught by bugprone-shared-ptr-array-mismatch checks (mismatched new/delete) } >From bc05c23dbb6606443045aeffb42bc526da60f3e4 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 17:36:49 +0300 Subject: [PATCH 05/14] Introduce options --- .../bugprone/SmartPtrInitializationCheck.cpp | 51 ++++++++++++++----- .../bugprone/SmartPtrInitializationCheck.h | 7 ++- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp index e925b382c277f2..89f698ccd38c4f 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -7,6 +7,7 @@ //===----------------------------------------------------------------------===// #include "SmartPtrInitializationCheck.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/AST/DeclCXX.h" #include "clang/ASTMatchers/ASTMatchFinder.h" @@ -18,35 +19,62 @@ namespace clang::tidy::bugprone { namespace { -// TODO: all types must be in config -// TODO: boost::shared_ptr and boost::unique_ptr -// TODO: reset and release must be in config +const auto DefaultSharedPointers = "::std::shared_ptr;::boost::shared_ptr"; +const auto DefaultUniquePointers = "::std::unique_ptr"; +const auto DefaultDefaultDeleters = "::std::default_delete"; } // namespace +SmartPtrInitializationCheck::SmartPtrInitializationCheck( + StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + SharedPointers(utils::options::parseStringList( + Options.get("SharedPointers", DefaultSharedPointers))), + UniquePointers(utils::options::parseStringList( + Options.get("UniquePointers", DefaultUniquePointers))), + DefaultDeleters(utils::options::parseStringList( + Options.get("DefaultDeleters", DefaultDefaultDeleters))) {} + +void SmartPtrInitializationCheck::storeOptions( + ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "SharedPointers", + utils::options::serializeStringList(SharedPointers)); + Options.store(Opts, "UniquePointers", + utils::options::serializeStringList(UniquePointers)); + Options.store(Opts, "DefaultDeleters", + utils::options::serializeStringList(DefaultDeleters)); +} + void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { auto ReleaseCallMatcher = cxxMemberCallExpr(callee(cxxMethodDecl(hasName("release")))); + // Build matchers for the smart pointer types + auto SharedPtrMatcher = hasAnyName(SharedPointers); + auto UniquePtrMatcher = hasAnyName(UniquePointers); + auto AllSmartPtrMatcher = anyOf(SharedPtrMatcher, UniquePtrMatcher); + + // Matcher for unique_ptr types with custom deleters + auto DefaultDeleterMatcher = hasAnyName(DefaultDeleters); auto UniquePtrWithCustomDeleter = classTemplateSpecializationDecl( - hasName("std::unique_ptr"), templateArgumentCountIs(2), - hasTemplateArgument(1, refersToType(unless(hasDeclaration(cxxRecordDecl( - hasName("std::default_delete"))))))); + UniquePtrMatcher, templateArgumentCountIs(2), + hasTemplateArgument(1, refersToType(unless(hasDeclaration(cxxRecordDecl( + DefaultDeleterMatcher)))))); // Matcher for smart pointer constructors // Exclude constructors with custom deleters: // - shared_ptr with 2+ arguments (second is deleter) // - unique_ptr with 2+ template args where second is not default_delete auto HasCustomDeleter = anyOf( - allOf(hasDeclaration( - cxxConstructorDecl(ofClass(hasName("std::shared_ptr")))), + allOf(hasDeclaration(cxxConstructorDecl( + ofClass(SharedPtrMatcher))), hasArgument(1, anything())), hasDeclaration(cxxConstructorDecl(ofClass(UniquePtrWithCustomDeleter)))); auto smartPtrConstructorMatcher = cxxConstructExpr( hasDeclaration(cxxConstructorDecl( - ofClass(hasAnyName("std::shared_ptr", "std::unique_ptr")), + ofClass(AllSmartPtrMatcher), unless(anyOf(isCopyConstructor(), isMoveConstructor())))), hasArgument(0, expr(unless(nullPointerConstant())).bind("pointer-arg")), @@ -60,14 +88,13 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { // - unique_ptr with custom deleter type (2+ template args where second is not // default_delete) auto HasCustomDeleterInReset = - anyOf(allOf(on(hasType(cxxRecordDecl(hasName("std::shared_ptr")))), + anyOf(allOf(on(hasType(cxxRecordDecl(SharedPtrMatcher))), hasArgument(1, anything())), on(hasType(qualType(hasDeclaration(UniquePtrWithCustomDeleter))))); auto resetCallMatcher = cxxMemberCallExpr( - on(hasType( - cxxRecordDecl(hasAnyName("std::shared_ptr", "std::unique_ptr")))), + on(hasType(cxxRecordDecl(AllSmartPtrMatcher))), callee(cxxMethodDecl(hasName("reset"))), hasArgument(0, expr(unless(nullPointerConstant())).bind("pointer-arg")), diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h index b21774b6b08068..d46a48242cf20d 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h @@ -20,14 +20,17 @@ namespace clang::tidy::bugprone { /// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/smart-ptr-initialization.html class SmartPtrInitializationCheck : public ClangTidyCheck { public: - SmartPtrInitializationCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + SmartPtrInitializationCheck(StringRef Name, ClangTidyContext *Context); void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: std::string getPointerDescription(const Expr *PointerExpr, ASTContext &Context); + const std::vector<StringRef> SharedPointers; + const std::vector<StringRef> UniquePointers; + const std::vector<StringRef> DefaultDeleters; }; } // namespace clang::tidy::bugprone >From f32db1c4f0babe2ad78b3be05b10233489dd314f Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 17:44:53 +0300 Subject: [PATCH 06/14] Refactoring the code following LLVM Code style --- .../bugprone/SmartPtrInitializationCheck.cpp | 45 +++++++++---------- .../bugprone/SmartPtrInitializationCheck.h | 4 +- 2 files changed, 24 insertions(+), 25 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp index 89f698ccd38c4f..10896e80ba80f0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -58,20 +58,19 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { auto DefaultDeleterMatcher = hasAnyName(DefaultDeleters); auto UniquePtrWithCustomDeleter = classTemplateSpecializationDecl( UniquePtrMatcher, templateArgumentCountIs(2), - hasTemplateArgument(1, refersToType(unless(hasDeclaration(cxxRecordDecl( - DefaultDeleterMatcher)))))); + hasTemplateArgument(1, refersToType(unless(hasDeclaration( + cxxRecordDecl(DefaultDeleterMatcher)))))); // Matcher for smart pointer constructors // Exclude constructors with custom deleters: // - shared_ptr with 2+ arguments (second is deleter) // - unique_ptr with 2+ template args where second is not default_delete auto HasCustomDeleter = anyOf( - allOf(hasDeclaration(cxxConstructorDecl( - ofClass(SharedPtrMatcher))), + allOf(hasDeclaration(cxxConstructorDecl(ofClass(SharedPtrMatcher))), hasArgument(1, anything())), hasDeclaration(cxxConstructorDecl(ofClass(UniquePtrWithCustomDeleter)))); - auto smartPtrConstructorMatcher = + auto SmartPtrConstructorMatcher = cxxConstructExpr( hasDeclaration(cxxConstructorDecl( ofClass(AllSmartPtrMatcher), @@ -92,7 +91,7 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { hasArgument(1, anything())), on(hasType(qualType(hasDeclaration(UniquePtrWithCustomDeleter))))); - auto resetCallMatcher = + auto ResetCallMatcher = cxxMemberCallExpr( on(hasType(cxxRecordDecl(AllSmartPtrMatcher))), callee(cxxMethodDecl(hasName("reset"))), @@ -102,41 +101,41 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { unless(hasArgument(0, ReleaseCallMatcher))) .bind("reset-call"); - Finder->addMatcher(smartPtrConstructorMatcher, this); - Finder->addMatcher(resetCallMatcher, this); + Finder->addMatcher(SmartPtrConstructorMatcher, this); + Finder->addMatcher(ResetCallMatcher, this); } void SmartPtrInitializationCheck::check( const MatchFinder::MatchResult &Result) { - const auto *pointerArg = Result.Nodes.getNodeAs<Expr>("pointer-arg"); - const auto *constructor = + const auto *PointerArg = Result.Nodes.getNodeAs<Expr>("pointer-arg"); + const auto *Constructor = Result.Nodes.getNodeAs<CXXConstructExpr>("constructor"); const auto *ResetCall = Result.Nodes.getNodeAs<CXXMemberCallExpr>("reset-call"); - assert(pointerArg); + assert(PointerArg); - const SourceLocation loc = pointerArg->getBeginLoc(); - const CXXMethodDecl *MD = - constructor ? constructor->getConstructor() + const SourceLocation Loc = PointerArg->getBeginLoc(); + const CXXMethodDecl *MethodDecl = + Constructor ? Constructor->getConstructor() : (ResetCall ? ResetCall->getMethodDecl() : nullptr); - if (!MD) + if (!MethodDecl) return; - const auto *record = MD->getParent(); - if (!record) + const auto *Record = MethodDecl->getParent(); + if (!Record) return; - const std::string typeName = record->getQualifiedNameAsString(); - diag(loc, "passing a raw pointer '%0' to %1%2 may cause double deletion") - << getPointerDescription(pointerArg, *Result.Context) << typeName - << (constructor ? " constructor" : "::reset()"); + const std::string TypeName = Record->getQualifiedNameAsString(); + diag(Loc, "passing a raw pointer '%0' to %1%2 may cause double deletion") + << getPointerDescription(PointerArg, *Result.Context) << TypeName + << (Constructor ? " constructor" : "::reset()"); } std::string SmartPtrInitializationCheck::getPointerDescription(const Expr *PointerExpr, ASTContext &Context) { - std::string Desc; - llvm::raw_string_ostream OS(Desc); + std::string Description; + llvm::raw_string_ostream OS(Description); // Try to get a readable representation of the expression PrintingPolicy Policy(Context.getLangOpts()); diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h index d46a48242cf20d..e8814387d0bc4b 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.h @@ -26,7 +26,7 @@ class SmartPtrInitializationCheck : public ClangTidyCheck { void storeOptions(ClangTidyOptions::OptionMap &Opts) override; private: - std::string getPointerDescription(const Expr *PointerExpr, + std::string getPointerDescription(const Expr *PointerExpr, ASTContext &Context); const std::vector<StringRef> SharedPointers; const std::vector<StringRef> UniquePointers; @@ -35,4 +35,4 @@ class SmartPtrInitializationCheck : public ClangTidyCheck { } // namespace clang::tidy::bugprone -#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRINITIALIZATIONCHECK_H \ No newline at end of file +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_SMARTPTRINITIALIZATIONCHECK_H >From b3670d28885f110a24b52ad90aac106cc08f7c07 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 17:48:54 +0300 Subject: [PATCH 07/14] remove redundant change --- clang-tools-extra/docs/clang-tidy/checks/list.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index aa4d8106eef9a1..a21d986e3a02b3 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -626,4 +626,4 @@ Check aliases :doc:`hicpp-use-override <hicpp/use-override>`, :doc:`modernize-use-override <modernize/use-override>`, "Yes" :doc:`hicpp-vararg <hicpp/vararg>`, :doc:`cppcoreguidelines-pro-type-vararg <cppcoreguidelines/pro-type-vararg>`, :doc:`llvm-else-after-return <llvm/else-after-return>`, :doc:`readability-else-after-return <readability/else-after-return>`, "Yes" - :doc:`llvm-qualified-auto <llvm/qualified-auto>`, :doc:`readability-qualified-auto <readability/qualified-auto>`, "Yes" \ No newline at end of file + :doc:`llvm-qualified-auto <llvm/qualified-auto>`, :doc:`readability-qualified-auto <readability/qualified-auto>`, "Yes" >From 189fbd3b25ae9a7d3f0751ba17cf2b69e5a1983b Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 18:07:52 +0300 Subject: [PATCH 08/14] Add alias in CERT module --- clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp | 3 +++ clang-tools-extra/docs/ReleaseNotes.rst | 5 +++++ .../docs/clang-tidy/checks/cert/mem56-cpp.rst | 10 ++++++++++ 3 files changed, 18 insertions(+) create mode 100644 clang-tools-extra/docs/clang-tidy/checks/cert/mem56-cpp.rst diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp index f64cb47d18b4ef..32d273c3745115 100644 --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -21,6 +21,7 @@ #include "../bugprone/SignalHandlerCheck.h" #include "../bugprone/SignedCharMisuseCheck.h" #include "../bugprone/SizeofExpressionCheck.h" +#include "../bugprone/SmartPtrInitializationCheck.h" #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" #include "../bugprone/StdNamespaceModificationCheck.h" #include "../bugprone/SuspiciousMemoryComparisonCheck.h" @@ -267,6 +268,8 @@ class CERTModule : public ClangTidyModule { CheckFactories.registerCheck<misc::ThrowByValueCatchByReferenceCheck>( "cert-err61-cpp"); // MEM + CheckFactories.registerCheck<bugprone::SmartPtrInitializationCheck>( + "cert-mem56-cpp"); CheckFactories .registerCheck<bugprone::DefaultOperatorNewOnOveralignedTypeCheck>( "cert-mem57-cpp"); diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 64098142025e40..ee3ec09825535c 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -142,6 +142,11 @@ New checks New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cert-mem56-cpp <clang-tidy/checks/cert/mem56-cpp>` to + :doc:`bugprone-smart-ptr-initialization + <clang-tidy/checks/bugprone/smart-ptr-initialization>` + was added. + Changes in existing checks ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert/mem56-cpp.rst b/clang-tools-extra/docs/clang-tidy/checks/cert/mem56-cpp.rst new file mode 100644 index 00000000000000..6858b4d1e4c11f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert/mem56-cpp.rst @@ -0,0 +1,10 @@ +.. title:: clang-tidy - cert-mem56-cpp +.. meta:: + :http-equiv=refresh: 5;URL=../bugprone/smart-ptr-initialization.html + +cert-mem56-cpp +============== + +The `cert-mem56-cpp` check is an alias, please see +:doc:`bugprone-smart-ptr-initialization.html +<../bugprone/smart-ptr-initialization.html>` for more information. >From 7d4a64c4c5daf9b8f8bf3bffac342c2e6a25b7e9 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 22:04:56 +0300 Subject: [PATCH 09/14] refactoring --- .../bugprone/SmartPtrInitializationCheck.cpp | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp index 10896e80ba80f0..24d590e5b9b96e 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -46,37 +46,48 @@ void SmartPtrInitializationCheck::storeOptions( } void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { + const auto IsSharedPtr = hasAnyName(SharedPointers); + const auto IsUniquePtr = hasAnyName(UniquePointers); + const auto IsSmartPtr = anyOf(IsSharedPtr, IsUniquePtr); + const auto IsDefaultDeleter = hasAnyName(DefaultDeleters); + + const auto IsSharedPtrRecord = cxxRecordDecl(IsSharedPtr); + const auto IsUniquePtrRecord = cxxRecordDecl(IsUniquePtr); + const auto IsSmartPtrRecord = cxxRecordDecl(IsSmartPtr); + + auto ReleaseMethod = cxxMethodDecl(hasName("release")); + auto ResetMethod = cxxMethodDecl(hasName("reset")); + auto ReleaseCallMatcher = - cxxMemberCallExpr(callee(cxxMethodDecl(hasName("release")))); + cxxMemberCallExpr(callee(ReleaseMethod)); - // Build matchers for the smart pointer types - auto SharedPtrMatcher = hasAnyName(SharedPointers); - auto UniquePtrMatcher = hasAnyName(UniquePointers); - auto AllSmartPtrMatcher = anyOf(SharedPtrMatcher, UniquePtrMatcher); + auto PointerArg = expr(unless(nullPointerConstant())).bind("pointer-arg"); // Matcher for unique_ptr types with custom deleters - auto DefaultDeleterMatcher = hasAnyName(DefaultDeleters); auto UniquePtrWithCustomDeleter = classTemplateSpecializationDecl( - UniquePtrMatcher, templateArgumentCountIs(2), - hasTemplateArgument(1, refersToType(unless(hasDeclaration( - cxxRecordDecl(DefaultDeleterMatcher)))))); + IsUniquePtr, templateArgumentCountIs(2), + hasTemplateArgument(1, refersToType(unless(hasUnqualifiedDesugaredType( + recordType(hasDeclaration( + classTemplateSpecializationDecl( + IsDefaultDeleter)))))))); // Matcher for smart pointer constructors // Exclude constructors with custom deleters: // - shared_ptr with 2+ arguments (second is deleter) // - unique_ptr with 2+ template args where second is not default_delete auto HasCustomDeleter = anyOf( - allOf(hasDeclaration(cxxConstructorDecl(ofClass(SharedPtrMatcher))), + allOf(hasDeclaration(cxxConstructorDecl(ofClass(IsSharedPtrRecord))), hasArgument(1, anything())), - hasDeclaration(cxxConstructorDecl(ofClass(UniquePtrWithCustomDeleter)))); + allOf(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(UniquePtrWithCustomDeleter)))), + hasDeclaration(cxxConstructorDecl(ofClass(IsUniquePtrRecord))))); auto SmartPtrConstructorMatcher = cxxConstructExpr( hasDeclaration(cxxConstructorDecl( - ofClass(AllSmartPtrMatcher), + ofClass(IsSmartPtrRecord), unless(anyOf(isCopyConstructor(), isMoveConstructor())))), - hasArgument(0, - expr(unless(nullPointerConstant())).bind("pointer-arg")), + hasArgument(0, PointerArg), unless(HasCustomDeleter), unless(hasArgument(0, cxxNewExpr())), unless(hasArgument(0, ReleaseCallMatcher))) .bind("constructor"); @@ -87,16 +98,20 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { // - unique_ptr with custom deleter type (2+ template args where second is not // default_delete) auto HasCustomDeleterInReset = - anyOf(allOf(on(hasType(cxxRecordDecl(SharedPtrMatcher))), + anyOf(allOf(on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration( + classTemplateSpecializationDecl(IsSharedPtr)))))), hasArgument(1, anything())), - on(hasType(qualType(hasDeclaration(UniquePtrWithCustomDeleter))))); + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(UniquePtrWithCustomDeleter)))))); auto ResetCallMatcher = cxxMemberCallExpr( - on(hasType(cxxRecordDecl(AllSmartPtrMatcher))), - callee(cxxMethodDecl(hasName("reset"))), - hasArgument(0, - expr(unless(nullPointerConstant())).bind("pointer-arg")), + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration( + classTemplateSpecializationDecl(IsSmartPtr)))))), + callee(ResetMethod), + hasArgument(0, PointerArg), unless(HasCustomDeleterInReset), unless(hasArgument(0, cxxNewExpr())), unless(hasArgument(0, ReleaseCallMatcher))) .bind("reset-call"); >From f75e966618bb7e2cefffa56ccf038860de607d5a Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 22:06:39 +0300 Subject: [PATCH 10/14] Fix && apply format --- .../bugprone/SmartPtrInitializationCheck.cpp | 44 +++++++++---------- 1 file changed, 20 insertions(+), 24 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp index 24d590e5b9b96e..786d949903d7a0 100644 --- a/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/SmartPtrInitializationCheck.cpp @@ -58,18 +58,19 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { auto ReleaseMethod = cxxMethodDecl(hasName("release")); auto ResetMethod = cxxMethodDecl(hasName("reset")); - auto ReleaseCallMatcher = - cxxMemberCallExpr(callee(ReleaseMethod)); + auto ReleaseCallMatcher = cxxMemberCallExpr(callee(ReleaseMethod)); - auto PointerArg = expr(unless(nullPointerConstant())).bind("pointer-arg"); + // Array automatically decays to pointer + auto PointerArg = expr(anyOf(hasType(pointerType()), hasType(arrayType()))) + .bind("pointer-arg"); // Matcher for unique_ptr types with custom deleters auto UniquePtrWithCustomDeleter = classTemplateSpecializationDecl( IsUniquePtr, templateArgumentCountIs(2), - hasTemplateArgument(1, refersToType(unless(hasUnqualifiedDesugaredType( - recordType(hasDeclaration( - classTemplateSpecializationDecl( - IsDefaultDeleter)))))))); + hasTemplateArgument( + 1, refersToType( + unless(hasUnqualifiedDesugaredType(recordType(hasDeclaration( + classTemplateSpecializationDecl(IsDefaultDeleter)))))))); // Matcher for smart pointer constructors // Exclude constructors with custom deleters: @@ -84,11 +85,9 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { auto SmartPtrConstructorMatcher = cxxConstructExpr( - hasDeclaration(cxxConstructorDecl( - ofClass(IsSmartPtrRecord), - unless(anyOf(isCopyConstructor(), isMoveConstructor())))), - hasArgument(0, PointerArg), - unless(HasCustomDeleter), unless(hasArgument(0, cxxNewExpr())), + hasDeclaration(cxxConstructorDecl(ofClass(IsSmartPtrRecord))), + hasArgument(0, PointerArg), unless(HasCustomDeleter), + unless(hasArgument(0, cxxNewExpr())), unless(hasArgument(0, ReleaseCallMatcher))) .bind("constructor"); @@ -97,21 +96,18 @@ void SmartPtrInitializationCheck::registerMatchers(MatchFinder *Finder) { // - shared_ptr with 2+ arguments (second is deleter) // - unique_ptr with custom deleter type (2+ template args where second is not // default_delete) - auto HasCustomDeleterInReset = - anyOf(allOf(on(hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration( - classTemplateSpecializationDecl(IsSharedPtr)))))), - hasArgument(1, anything())), - on(hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration(UniquePtrWithCustomDeleter)))))); + auto HasCustomDeleterInReset = anyOf( + allOf(on(hasType(hasUnqualifiedDesugaredType(recordType(hasDeclaration( + classTemplateSpecializationDecl(IsSharedPtr)))))), + hasArgument(1, anything())), + on(hasType(hasUnqualifiedDesugaredType( + recordType(hasDeclaration(UniquePtrWithCustomDeleter)))))); auto ResetCallMatcher = cxxMemberCallExpr( - on(hasType(hasUnqualifiedDesugaredType( - recordType(hasDeclaration( - classTemplateSpecializationDecl(IsSmartPtr)))))), - callee(ResetMethod), - hasArgument(0, PointerArg), + on(hasType(hasUnqualifiedDesugaredType(recordType( + hasDeclaration(classTemplateSpecializationDecl(IsSmartPtr)))))), + callee(ResetMethod), hasArgument(0, PointerArg), unless(HasCustomDeleterInReset), unless(hasArgument(0, cxxNewExpr())), unless(hasArgument(0, ReleaseCallMatcher))) .bind("reset-call"); >From e141347c5980211a976267e8bbd8049feebf07c1 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 23:08:20 +0300 Subject: [PATCH 11/14] Docs by DeepSeek --- clang-tools-extra/docs/ReleaseNotes.rst | 3 +- .../bugprone/smart-ptr-initialization.rst | 104 +++++++++++++++++- 2 files changed, 104 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ee3ec09825535c..e3c3ea6cc648d3 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -100,7 +100,8 @@ New checks - New :doc:`bugprone-smart-ptr-initialization <clang-tidy/checks/bugprone/smart-ptr-initialization>` check. - FIXME: Write a short description. + Detects dangerous initialization of smart pointers with raw pointers that are + already owned elsewhere, which can lead to double deletion. - New :doc:`llvm-type-switch-case-types <clang-tidy/checks/llvm/type-switch-case-types>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst index ed72c6b1b92b11..b3b4bb9ca60e70 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst @@ -1,6 +1,106 @@ .. title:: clang-tidy - bugprone-smart-ptr-initialization bugprone-smart-ptr-initialization -================================= +================================== -FIXME: Describe what patterns does the check detect and why. Give examples. +Detects dangerous initialization of smart pointers with raw pointers that are +already owned elsewhere, which can lead to double deletion. + +This check implements CERT C++ rule `MEM56-CPP. Do not store an already-owned +pointer value in an unrelated smart pointer +<https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM56-CPP.+Do+not+store+an+already-owned+pointer+value+in+an+unrelated+smart+pointer>`_. + +Examples +-------- + +The check flags cases where raw pointers that are already owned or managed +elsewhere are passed to smart pointer constructors or ``reset()`` methods: + +.. code-block:: c++ + + A& getA(); + void foo() { + // Warning: '&getA()' is already managed elsewhere + std::shared_ptr<A> a(&getA()); + } + + void bar() { + int x = 10; + // Warning: '&x' points to a local variable + std::unique_ptr<int> ptr(&x); + } + + void baz() { + std::vector<int> vec{1, 2, 3}; + std::shared_ptr<int> sp; + // Warning: '&vec[0]' is managed by the vector + sp.reset(&vec[0]); + } + +Allowed cases +------------- + +The check ignores legitimate cases: + +1. **New expressions**: Pointers from ``new`` operators are safe: + + .. code-block:: c++ + + std::unique_ptr<int> p(new int(5)); // OK + +2. **Release calls**: Pointers from ``release()`` method are transferred: + + .. code-block:: c++ + + auto p1 = std::make_unique<int>(5); + std::unique_ptr<int> p2(p1.release()); // OK + +3. **Custom deleters**: Smart pointers with custom deleters are ignored: + + .. code-block:: c++ + + void customDeleter(int* p) { delete p; } + std::unique_ptr<int, decltype(&customDeleter)> p(&getA(), customDeleter); + +4. **Null pointers**: ``nullptr`` is always safe: + + .. code-block:: c++ + + std::shared_ptr<int> p(nullptr); // OK + p.reset(nullptr); // OK + +Options +------- + +.. option:: SharedPointers + + A semicolon-separated list of (fully qualified) shared pointer type names + that should be checked. Default value is + `::std::shared_ptr;::boost::shared_ptr`. + +.. option:: UniquePointers + + A semicolon-separated list of (fully qualified) unique pointer type names + that should be checked. Default value is + `::std::unique_ptr`. + +.. option:: DefaultDeleters + + A semicolon-separated list of (fully qualified) default deleter type names. + Smart pointers with deleters matching these types are considered to use the + default deleter and are checked. Smart pointers with custom deleters are + ignored. Default value is `::std::default_delete`. + +Limitations +---------- + +This check only supports smart pointers with shared and unique ownership +semantics. Smart pointers with different semantics, such as +``boost::scoped_ptr``, cannot be used with the current version of this check. + +References +---------- + +* `CERT C++ MEM56-CPP <https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM56-CPP.+Do+not+store+an+already-owned+pointer+value+in+an+unrelated+smart+pointer>`_ +* `C++ Core Guidelines R.3: A raw pointer (a T*) is non-owning <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr>`_ +* `C++ Core Guidelines R.20: Use unique_ptr or shared_ptr to represent ownership <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-owner>`_ >From 4f77a71e80faabda753e8023db6fb9e125d2b732 Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 23:20:05 +0300 Subject: [PATCH 12/14] fix links --- .../clang-tidy/checks/bugprone/smart-ptr-initialization.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst index b3b4bb9ca60e70..f50ac44f7d1095 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst @@ -102,5 +102,5 @@ References ---------- * `CERT C++ MEM56-CPP <https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM56-CPP.+Do+not+store+an+already-owned+pointer+value+in+an+unrelated+smart+pointer>`_ -* `C++ Core Guidelines R.3: A raw pointer (a T*) is non-owning <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-ptr>`_ -* `C++ Core Guidelines R.20: Use unique_ptr or shared_ptr to represent ownership <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rr-owner>`_ +* `C++ Core Guidelines R.3: A raw pointer (a T*) is non-owning <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r20-use-unique_ptr-or-shared_ptr-to-represent-ownership>`_ +* `C++ Core Guidelines R.20: Use unique_ptr or shared_ptr to represent ownership <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r20-use-unique_ptr-or-shared_ptr-to-represent-ownership>`_ >From 4f19e1fbbd22d19aeee7e67a6a3b2d0a4dd417ee Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 23:23:05 +0300 Subject: [PATCH 13/14] fix link --- .../clang-tidy/checks/bugprone/smart-ptr-initialization.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst index f50ac44f7d1095..fc0249e1979616 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst @@ -102,5 +102,5 @@ References ---------- * `CERT C++ MEM56-CPP <https://wiki.sei.cmu.edu/confluence/display/cplusplus/MEM56-CPP.+Do+not+store+an+already-owned+pointer+value+in+an+unrelated+smart+pointer>`_ -* `C++ Core Guidelines R.3: A raw pointer (a T*) is non-owning <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r20-use-unique_ptr-or-shared_ptr-to-represent-ownership>`_ +* `C++ Core Guidelines R.3: A raw pointer (a T*) is non-owning <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r3-a-raw-pointer-a-t-is-non-owning>`_ * `C++ Core Guidelines R.20: Use unique_ptr or shared_ptr to represent ownership <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#r20-use-unique_ptr-or-shared_ptr-to-represent-ownership>`_ >From 5ec9e69a4f88f811af9f3a6dace246c01c3d170d Mon Sep 17 00:00:00 2001 From: denzor200 <[email protected]> Date: Sun, 15 Feb 2026 23:47:43 +0300 Subject: [PATCH 14/14] lint --- .../clang-tidy/checks/bugprone/smart-ptr-initialization.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst index fc0249e1979616..f1cce7e97c0fdb 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/smart-ptr-initialization.rst @@ -92,7 +92,7 @@ Options ignored. Default value is `::std::default_delete`. Limitations ----------- +----------- This check only supports smart pointers with shared and unique ownership semantics. Smart pointers with different semantics, such as _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
