caio/clang-p2996
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[clang-tidy] Create bugprone-incorrect-enable-shared-from-this check (#102299)

Browse Source 
This checks that classes/structs inheriting from
``std::enable_shared_from_this`` does so with public inheritance, so it
prevents crashes due to ``std::make_shared`` and ``shared_from_this()``
getting called when the internal weak pointer was not initialized (e.g.
due to private inheritance).
This commit is contained in:
MichelleCDjunaidi
2025-01-12 18:04:40 +08:00
committed by GitHub
parent fdfe7e7fab
commit 8ebc35f8d0
8 changed files with 330 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
View File

@@ -33,6 +33,7 @@
#include "InaccurateEraseCheck.h"
#include "IncDecInConditionsCheck.h"
#include "IncorrectEnableIfCheck.h"
#include "IncorrectEnableSharedFromThisCheck.h"
#include "IncorrectRoundingsCheck.h"
#include "InfiniteLoopCheck.h"
#include "IntegerDivisionCheck.h"
@@ -144,6 +145,8 @@ public:
"bugprone-inaccurate-erase");
CheckFactories.registerCheck<IncorrectEnableIfCheck>(
"bugprone-incorrect-enable-if");
CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(
"bugprone-incorrect-enable-shared-from-this");
CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
"bugprone-return-const-ref-from-parameter");
CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(

5
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
View File

@@ -27,6 +27,11 @@ add_clang_library(clangTidyBugproneModule STATIC
ForwardingReferenceOverloadCheck.cpp
ImplicitWideningOfMultiplicationResultCheck.cpp
InaccurateEraseCheck.cpp
IncorrectEnableIfCheck.cpp
IncorrectEnableSharedFromThisCheck.cpp
ReturnConstRefFromParameterCheck.cpp
SuspiciousStringviewDataUsageCheck.cpp
SwitchMissingDefaultCaseCheck.cpp
IncDecInConditionsCheck.cpp
IncorrectEnableIfCheck.cpp
IncorrectRoundingsCheck.cpp

65
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.cpp Normal file
View File

@@ -0,0 +1,65 @@
//===--- IncorrectEnableSharedFromThisCheck.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 "IncorrectEnableSharedFromThisCheck.h"
#include "clang/AST/ASTContext.h"
#include "clang/AST/DeclCXX.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
using namespace clang::ast_matchers;
namespace clang::tidy::bugprone {
void IncorrectEnableSharedFromThisCheck::registerMatchers(MatchFinder *Finder) {
const auto EnableSharedFromThis =
cxxRecordDecl(hasName("enable_shared_from_this"), isInStdNamespace());
const auto QType = hasCanonicalType(hasDeclaration(
cxxRecordDecl(
anyOf(EnableSharedFromThis.bind("enable_rec"),
cxxRecordDecl(hasAnyBase(cxxBaseSpecifier(
isPublic(), hasType(hasCanonicalType(
hasDeclaration(EnableSharedFromThis))))))))
.bind("base_rec")));
Finder->addMatcher(
cxxRecordDecl(
unless(isExpansionInSystemHeader()),
hasDirectBase(cxxBaseSpecifier(unless(isPublic()), hasType(QType))
.bind("base")))
.bind("derived"),
this);
}
void IncorrectEnableSharedFromThisCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *BaseSpec = Result.Nodes.getNodeAs<CXXBaseSpecifier>("base");
const auto *Base = Result.Nodes.getNodeAs<CXXRecordDecl>("base_rec");
const auto *Derived = Result.Nodes.getNodeAs<CXXRecordDecl>("derived");
const bool IsEnableSharedFromThisDirectBase =
Result.Nodes.getNodeAs<CXXRecordDecl>("enable_rec") == Base;
const bool HasWrittenAccessSpecifier =
BaseSpec->getAccessSpecifierAsWritten() != AS_none;
const auto ReplacementRange = CharSourceRange(
SourceRange(BaseSpec->getBeginLoc()), HasWrittenAccessSpecifier);
const llvm::StringRef Replacement =
HasWrittenAccessSpecifier ? "public" : "public ";
const FixItHint Hint =
IsEnableSharedFromThisDirectBase
? FixItHint::CreateReplacement(ReplacementRange, Replacement)
: FixItHint();
diag(Derived->getLocation(),
"%2 is not publicly inheriting from "
"%select{%1 which inherits from |}0'std::enable_shared_"
"from_this', "
"which will cause unintended behaviour "
"when using 'shared_from_this'; make the inheritance "
"public",
DiagnosticIDs::Warning)
<< IsEnableSharedFromThisDirectBase << Base << Derived << Hint;
}
} // namespace clang::tidy::bugprone

35
clang-tools-extra/clang-tidy/bugprone/IncorrectEnableSharedFromThisCheck.h Normal file
View File

@@ -0,0 +1,35 @@
//===--- IncorrectEnableSharedFromThisCheck.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_INCORRECTENABLESHAREDFROMTHISCHECK_H
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H
#include "../ClangTidyCheck.h"
namespace clang::tidy::bugprone {
/// Detect classes or structs that do not publicly inherit from
/// ``std::enable_shared_from_this``, because unintended behavior will
/// otherwise occur when calling ``shared_from_this``.
///
/// For the user-facing documentation see:
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.html
class IncorrectEnableSharedFromThisCheck : public ClangTidyCheck {
public:
IncorrectEnableSharedFromThisCheck(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.CPlusPlus11;
}
};
} // namespace clang::tidy::bugprone
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_INCORRECTENABLESHAREDFROMTHISCHECK_H

7
clang-tools-extra/docs/ReleaseNotes.rst
View File

@@ -145,6 +145,13 @@ New checks
Warns about code that tries to cast between pointers by means of
``std::bit_cast`` or ``memcpy``.
- New :doc:`bugprone-incorrect-enable-shared-from-this
<clang-tidy/checks/bugprone/incorrect-enable-shared-from-this>` check.
Detect classes or structs that do not publicly inherit from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.
- New :doc:`bugprone-nondeterministic-pointer-iteration-order
<clang-tidy/checks/bugprone/nondeterministic-pointer-iteration-order>`
check.

34
clang-tools-extra/docs/clang-tidy/checks/bugprone/incorrect-enable-shared-from-this.rst Normal file
View File

@@ -0,0 +1,34 @@
.. title:: clang-tidy - bugprone-incorrect-enable-shared-from-this
bugprone-incorrect-enable-shared-from-this
==========================================
Detect classes or structs that do not publicly inherit from
``std::enable_shared_from_this``, because unintended behavior will
otherwise occur when calling ``shared_from_this``.
Consider the following code:
.. code-block:: c++
#include <memory>
// private inheritance
class BadExample : std::enable_shared_from_this<BadExample> {
// ``shared_from_this``` unintended behaviour
// `libstdc++` implementation returns uninitialized ``weak_ptr``
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();
}
Using `libstdc++` implementation, ``shared_from_this`` will throw
``std::bad_weak_ptr``. When ``using_not_public()`` is called, this code will
crash without exception handling.

1
clang-tools-extra/docs/clang-tidy/checks/list.rst
View File

@@ -101,6 +101,7 @@ Clang-Tidy Checks
:doc:`bugprone-inaccurate-erase <bugprone/inaccurate-erase>`, "Yes"
:doc:`bugprone-inc-dec-in-conditions <bugprone/inc-dec-in-conditions>`,
:doc:`bugprone-incorrect-enable-if <bugprone/incorrect-enable-if>`, "Yes"
:doc:`bugprone-incorrect-enable-shared-from-this <bugprone/incorrect-enable-shared-from-this>`, "Yes"
:doc:`bugprone-incorrect-roundings <bugprone/incorrect-roundings>`,
:doc:`bugprone-infinite-loop <bugprone/infinite-loop>`,
:doc:`bugprone-integer-division <bugprone/integer-division>`,

180
clang-tools-extra/test/clang-tidy/checkers/bugprone/incorrect-enable-shared-from-this.cpp Normal file
View File

@@ -0,0 +1,180 @@
// RUN: %check_clang_tidy -std=c++11-or-later %s bugprone-incorrect-enable-shared-from-this %t
// NOLINTBEGIN
namespace std {
template <typename T> class enable_shared_from_this {};
} //namespace std
// NOLINTEND
class BadClassExample : std::enable_shared_from_this<BadClassExample> {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadClassExample' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadClassExample>
class BadClass2Example : private std::enable_shared_from_this<BadClass2Example> {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadClass2Example' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadClass2Example>
struct BadStructExample : private std::enable_shared_from_this<BadStructExample> {};
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'BadStructExample' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadStructExample>
class GoodClassExample : public std::enable_shared_from_this<GoodClassExample> {};
struct GoodStructExample : public std::enable_shared_from_this<GoodStructExample> {};
struct GoodStruct2Example : std::enable_shared_from_this<GoodStruct2Example> {};
class dummy_class1 {};
class dummy_class2 {};
class BadMultiClassExample : std::enable_shared_from_this<BadMultiClassExample>, dummy_class1 {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadMultiClassExample' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: public std::enable_shared_from_this<BadMultiClassExample>, dummy_class1
class BadMultiClass2Example : dummy_class1, std::enable_shared_from_this<BadMultiClass2Example>, dummy_class2 {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadMultiClass2Example' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: dummy_class1, public std::enable_shared_from_this<BadMultiClass2Example>, dummy_class2
class BadMultiClass3Example : dummy_class1, dummy_class2, std::enable_shared_from_this<BadMultiClass3Example> {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'BadMultiClass3Example' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: dummy_class1, dummy_class2, public std::enable_shared_from_this<BadMultiClass3Example>
class ClassBase : public std::enable_shared_from_this<ClassBase> {};
class PrivateInheritClassBase : private ClassBase {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateInheritClassBase' is not publicly inheriting from 'ClassBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
class DefaultInheritClassBase : ClassBase {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultInheritClassBase' is not publicly inheriting from 'ClassBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
class PublicInheritClassBase : public ClassBase {};
struct StructBase : public std::enable_shared_from_this<StructBase> {};
struct PrivateInheritStructBase : private StructBase {};
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateInheritStructBase' is not publicly inheriting from 'StructBase' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
struct DefaultInheritStructBase : StructBase {};
struct PublicInheritStructBase : StructBase {};
//alias the template itself
template <typename T> using esft_template = std::enable_shared_from_this<T>;
class PrivateAliasTemplateClassBase : private esft_template<PrivateAliasTemplateClassBase> {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateAliasTemplateClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateAliasTemplateClassBase : public esft_template<PrivateAliasTemplateClassBase> {};
class DefaultAliasTemplateClassBase : esft_template<DefaultAliasTemplateClassBase> {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultAliasTemplateClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultAliasTemplateClassBase : public esft_template<DefaultAliasTemplateClassBase> {};
class PublicAliasTemplateClassBase : public esft_template<PublicAliasTemplateClassBase> {};
struct PrivateAliasTemplateStructBase : private esft_template<PrivateAliasTemplateStructBase> {};
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateAliasTemplateStructBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateAliasTemplateStructBase : public esft_template<PrivateAliasTemplateStructBase> {};
struct DefaultAliasTemplateStructBase : esft_template<DefaultAliasTemplateStructBase> {};
struct PublicAliasTemplateStructBase : public esft_template<PublicAliasTemplateStructBase> {};
//alias with specific instance
using esft = std::enable_shared_from_this<ClassBase>;
class PrivateAliasClassBase : private esft {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateAliasClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateAliasClassBase : public esft {};
class DefaultAliasClassBase : esft {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultAliasClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultAliasClassBase : public esft {};
class PublicAliasClassBase : public esft {};
struct PrivateAliasStructBase : private esft {};
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateAliasStructBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateAliasStructBase : public esft {};
struct DefaultAliasStructBase : esft {};
struct PublicAliasStructBase : public esft {};
//we can only typedef a specific instance of the template
typedef std::enable_shared_from_this<ClassBase> EnableSharedFromThis;
class PrivateTypedefClassBase : private EnableSharedFromThis {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'PrivateTypedefClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class PrivateTypedefClassBase : public EnableSharedFromThis {};
class DefaultTypedefClassBase : EnableSharedFromThis {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'DefaultTypedefClassBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class DefaultTypedefClassBase : public EnableSharedFromThis {};
class PublicTypedefClassBase : public EnableSharedFromThis {};
struct PrivateTypedefStructBase : private EnableSharedFromThis {};
// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: 'PrivateTypedefStructBase' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: struct PrivateTypedefStructBase : public EnableSharedFromThis {};
struct DefaultTypedefStructBase : EnableSharedFromThis {};
struct PublicTypedefStructBase : public EnableSharedFromThis {};
#define PRIVATE_ESFT_CLASS(ClassName) \
class ClassName: private std::enable_shared_from_this<ClassName> { \
};
PRIVATE_ESFT_CLASS(PrivateEsftClass);
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'PrivateEsftClass' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
#define DEFAULT_ESFT_CLASS(ClassName) \
class ClassName: std::enable_shared_from_this<ClassName> { \
};
DEFAULT_ESFT_CLASS(DefaultEsftClass);
// CHECK-MESSAGES: :[[@LINE-1]]:20: warning: 'DefaultEsftClass' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
#define PUBLIC_ESFT_CLASS(ClassName) \
class ClassName: public std::enable_shared_from_this<ClassName> { \
};
PUBLIC_ESFT_CLASS(PublicEsftClass);
#define PRIVATE_ESFT_STRUCT(StructName) \
struct StructName: private std::enable_shared_from_this<StructName> { \
};
PRIVATE_ESFT_STRUCT(PrivateEsftStruct);
// CHECK-MESSAGES: :[[@LINE-1]]:21: warning: 'PrivateEsftStruct' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
#define DEFAULT_ESFT_STRUCT(StructName) \
struct StructName: std::enable_shared_from_this<StructName> { \
};
DEFAULT_ESFT_STRUCT(DefaultEsftStruct);
#define PUBLIC_ESFT_STRUCT(StructName) \
struct StructName: std::enable_shared_from_this<StructName> { \
};
PUBLIC_ESFT_STRUCT(PublicEsftStruct);
struct A : std::enable_shared_from_this<A> {};
#define MACRO_A A
class B : MACRO_A {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'B' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
class C : private MACRO_A {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'C' is not publicly inheriting from 'A' which inherits from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
class D : public MACRO_A {};
#define MACRO_PARAM(CLASS) std::enable_shared_from_this<CLASS>
class E : MACRO_PARAM(E) {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'E' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class E : public MACRO_PARAM(E) {};
class F : private MACRO_PARAM(F) {};
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: 'F' is not publicly inheriting from 'std::enable_shared_from_this', which will cause unintended behaviour when using 'shared_from_this'; make the inheritance public [bugprone-incorrect-enable-shared-from-this]
// CHECK-FIXES: class F : public MACRO_PARAM(F) {};
class G : public MACRO_PARAM(G) {};