https://github.com/MichelleCDjunaidi created https://github.com/llvm/llvm-project/pull/102299
This checks that classes/structs inheriting from ``std::enable_shared_from_this`` derive it with the ``public`` access specifier, so it prevents crashes due to ``std::make_shared`` and ``shared_from_this()`` getting called on a non-public class/struct. >From 75306bd83eb43d0606630f9f059fc04ad1b20b06 Mon Sep 17 00:00:00 2001 From: Michelle C Djunaidi <michellechrisa...@gmail.com> Date: Wed, 7 Aug 2024 13:10:02 +0800 Subject: [PATCH 1/2] [clang-tidy] Add bugprone-public-enable-shared-from-this check This check identifies classes deriving from std::enable_shared_from_this that does not inherit with the public keyword, which may cause problems when std::make_shared is called for that class. --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../PublicEnableSharedFromThisCheck.cpp | 45 +++++++++++++++ .../PublicEnableSharedFromThisCheck.h | 33 +++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 5 ++ .../bugprone/public-enable-shared-from-this | 6 ++ .../docs/clang-tidy/checks/list.rst | 1 + .../public-enable-shared-from-this.cpp | 56 +++++++++++++++++++ 8 files changed, 150 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 689eb92a3d8d17..f12f0cd1c47da3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -53,6 +53,7 @@ #include "ParentVirtualCallCheck.h" #include "PointerArithmeticOnPolymorphicObjectCheck.h" #include "PosixReturnCheck.h" +#include "PublicEnableSharedFromThisCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" #include "ReturnConstRefFromParameterCheck.h" @@ -139,6 +140,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); + CheckFactories.registerCheck<PublicEnableSharedFromThisCheck>( + "bugprone-public-enable-shared-from-this"); CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index cb0d8ae98bac58..c9bea094241edc 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule ImplicitWideningOfMultiplicationResultCheck.cpp InaccurateEraseCheck.cpp IncorrectEnableIfCheck.cpp + PublicEnableSharedFromThisCheck.cpp ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp new file mode 100644 index 00000000000000..dab3e0ac596fe7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.cpp @@ -0,0 +1,45 @@ +//===--- PublicEnableSharedFromThisCheck.cpp - clang-tidy -------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "PublicEnableSharedFromThisCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + + void PublicEnableSharedFromThisCheck::registerMatchers(MatchFinder *match_finder) { + match_finder->addMatcher( + cxxRecordDecl( + hasAnyBase( + cxxBaseSpecifier(unless(isPublic()), + hasType( + cxxRecordDecl( + hasName("::std::enable_shared_from_this")))) + ) + ) + .bind("not-public-enable-shared"), this); + } + + void PublicEnableSharedFromThisCheck::check(const MatchFinder::MatchResult &result) { + const auto *EnableSharedClassDecl = + result.Nodes.getNodeAs<CXXRecordDecl>("not-public-enable-shared"); + + for (const auto &Base : EnableSharedClassDecl->bases()) { + const auto *BaseType = Base.getType()->getAsCXXRecordDecl(); + if (BaseType && BaseType->getQualifiedNameAsString() == "std::enable_shared_from_this") { + SourceLocation InsertLoc = Base.getBeginLoc(); + FixItHint Hint = FixItHint::CreateInsertion(InsertLoc, "public "); + diag(EnableSharedClassDecl->getLocation(), "class %0 is not public even though it's derived from std::enable_shared_from_this", DiagnosticIDs::Warning) + << EnableSharedClassDecl->getNameAsString() + << Hint; + break; + } + } + } +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h new file mode 100644 index 00000000000000..8a4013c3a46b7f --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/PublicEnableSharedFromThisCheck.h @@ -0,0 +1,33 @@ +//===--- PublicEnableSharedFromThisCheck.h - clang-tidy ---------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/public-enable-shared-from-this.html +class PublicEnableSharedFromThisCheck : public ClangTidyCheck { +public: + PublicEnableSharedFromThisCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_PUBLICENABLESHAREDFROMTHISCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 642ad39cc0c1c5..67f6b1275a1adf 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -98,6 +98,11 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-public-enable-shared-from-this + <clang-tidy/checks/bugprone/public-enable-shared-from-this>` check. + + Checks if a class deriving from enable_shared_from_this has public access specifiers, because otherwise the program will crash when shared_from_this is called. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this new file mode 100644 index 00000000000000..d3f500eaa51460 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this @@ -0,0 +1,6 @@ +.. title:: clang-tidy - bugprone-public-enable-shared-from-this + +bugprone-public-enable-shared-from-this +======================================= + +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 a931ebf025a10e..2b918de89f7c19 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -120,6 +120,7 @@ Clang-Tidy Checks :doc:`bugprone-parent-virtual-call <bugprone/parent-virtual-call>`, "Yes" :doc:`bugprone-pointer-arithmetic-on-polymorphic-object <bugprone/pointer-arithmetic-on-polymorphic-object>`, :doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes" + :doc:`bugprone-public-enable-shared-from-this <bugprone/public-enable-shared-from-this>`, "Yes" :doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes" :doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes" :doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp new file mode 100644 index 00000000000000..a3e6d0ee6ca952 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp @@ -0,0 +1,56 @@ +// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- -I %S/Inputs/ +#include <memory> + +class BadExample : std::enable_shared_from_this<BadExample> { +// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] +// CHECK-FIXES: :[[@LINE-2]]:19 public + public: + BadExample* foo() { return shared_from_this().get(); } + void bar() { return; } +}; + +void using_not_public() { + auto bad_example = std::make_shared<BadExample>(); + auto* b_ex = bad_example->foo(); + b_ex->bar(); +} + +class GoodExample : public std::enable_shared_from_this<GoodExample> { + public: + GoodExample* foo() { return shared_from_this().get(); } + void bar() { return; } +}; + +void using_public() { + auto good_example = std::make_shared<GoodExample>(); + auto* g_ex = good_example->foo(); + g_ex->bar(); +} + +struct BaseClass { + + void print() { + (void) State; + (void) Requester; + } + bool State; + int Requester; +}; + +class InheritPrivateBaseClass : BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; +}; + +class InheritPublicBaseClass : public BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; +}; \ No newline at end of file >From 4461045503c3efc2170da750911c20d83afdb0f1 Mon Sep 17 00:00:00 2001 From: Michelle C Djunaidi <michellechrisa...@gmail.com> Date: Wed, 7 Aug 2024 17:58:42 +0800 Subject: [PATCH 2/2] Fix test to be compatible with llvm-lit, add docs --- .../bugprone/public-enable-shared-from-this | 6 -- .../public-enable-shared-from-this.rst | 26 ++++++ .../public-enable-shared-from-this.cpp | 93 ++++++++----------- 3 files changed, 67 insertions(+), 58 deletions(-) delete mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this deleted file mode 100644 index d3f500eaa51460..00000000000000 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this +++ /dev/null @@ -1,6 +0,0 @@ -.. title:: clang-tidy - bugprone-public-enable-shared-from-this - -bugprone-public-enable-shared-from-this -======================================= - -FIXME: Describe what patterns does the check detect and why. Give examples. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst new file mode 100644 index 00000000000000..616b2b52271e3f --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/public-enable-shared-from-this.rst @@ -0,0 +1,26 @@ +.. title:: clang-tidy - bugprone-public-enable-shared-from-this + +bugprone-public-enable-shared-from-this +======================================= + +Checks that classes/structs inheriting from ``std::enable_shared_from_this`` derive it with the ``public`` access specifier. If not, then issue a FixItHint that can be applied. + +Consider the following code: +.. code-block:: c++ + #include <memory> + + class BadExample : std::enable_shared_from_this<BadExample> { + // warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] + // will insert the public keyword if -fix is applied + public: + BadExample* foo() { return shared_from_this().get(); } + void bar() { return; } + }; + + void using_not_public() { + auto bad_example = std::make_shared<BadExample>(); + auto* b_ex = bad_example->foo(); + b_ex->bar(); + } + +When ``using_not_public()`` is called, this code will crash. diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp index a3e6d0ee6ca952..b0954898ddb783 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/public-enable-shared-from-this.cpp @@ -1,56 +1,45 @@ -// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- -I %S/Inputs/ -#include <memory> +// RUN: %check_clang_tidy %s bugprone-public-enable-shared-from-this %t -- -- -class BadExample : std::enable_shared_from_this<BadExample> { -// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] -// CHECK-FIXES: :[[@LINE-2]]:19 public - public: - BadExample* foo() { return shared_from_this().get(); } - void bar() { return; } -}; +namespace std { -void using_not_public() { - auto bad_example = std::make_shared<BadExample>(); - auto* b_ex = bad_example->foo(); - b_ex->bar(); -} + template <typename T> class enable_shared_from_this {}; -class GoodExample : public std::enable_shared_from_this<GoodExample> { - public: - GoodExample* foo() { return shared_from_this().get(); } - void bar() { return; } -}; - -void using_public() { - auto good_example = std::make_shared<GoodExample>(); - auto* g_ex = good_example->foo(); - g_ex->bar(); -} - -struct BaseClass { - - void print() { - (void) State; - (void) Requester; - } - bool State; - int Requester; -}; - -class InheritPrivateBaseClass : BaseClass { - public: - void additionalFunction() { - (void) ID; - } - private: - int ID; -}; - -class InheritPublicBaseClass : public BaseClass { - public: - void additionalFunction() { - (void) ID; + class BadExample : enable_shared_from_this<BadExample> {}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class BadExample is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] + // CHECK-FIXES: public enable_shared_from_this<BadExample> + + class Bad2Example : std::enable_shared_from_this<Bad2Example> {}; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: class Bad2Example is not public even though it's derived from std::enable_shared_from_this [bugprone-public-enable-shared-from-this] + // CHECK-FIXES: public std::enable_shared_from_this<Bad2Example> + + class GoodExample : public enable_shared_from_this<GoodExample> { + }; + + struct BaseClass { + + void print() { + (void) State; + (void) Requester; } - private: - int ID; -}; \ No newline at end of file + bool State; + int Requester; + }; + + class InheritPrivateBaseClass : BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; + }; + + class InheritPublicBaseClass : public BaseClass { + public: + void additionalFunction() { + (void) ID; + } + private: + int ID; + }; +} \ No newline at end of file _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits