https://github.com/HerrCai0907 updated https://github.com/llvm/llvm-project/pull/89497
>From 91915f68902ade86c0bf8eba643428017ae8bb3c Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Sat, 20 Apr 2024 17:58:19 +0800 Subject: [PATCH 1/3] [tidy] add new check bugprone-return-const-ref-from-parameter Detects the function which returns the const reference from parameter which causes potential use after free if the caller uses xvalue as argument. In c++, const reference parameter can accept xvalue which will be destructed after the call. When the function returns this parameter also as const reference, then the returned reference can be used after the object it refers to has been destroyed. Fixes: #85253 --- .../bugprone/BugproneTidyModule.cpp | 3 ++ .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../ReturnConstRefFromParameterCheck.cpp | 41 +++++++++++++++++++ .../ReturnConstRefFromParameterCheck.h | 35 ++++++++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 +++ .../return-const-ref-from-parameter.rst | 30 ++++++++++++++ .../docs/clang-tidy/checks/list.rst | 9 ++-- .../return-const-ref-from-parameter.cpp | 31 ++++++++++++++ 8 files changed, 152 insertions(+), 4 deletions(-) create mode 100644 clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 2931325d8b5798..1b92d2e60cc173 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -54,6 +54,7 @@ #include "PosixReturnCheck.h" #include "RedundantBranchConditionCheck.h" #include "ReservedIdentifierCheck.h" +#include "ReturnConstRefFromParameterCheck.h" #include "SharedPtrArrayMismatchCheck.h" #include "SignalHandlerCheck.h" #include "SignedCharMisuseCheck.h" @@ -137,6 +138,8 @@ class BugproneModule : public ClangTidyModule { "bugprone-inaccurate-erase"); CheckFactories.registerCheck<IncorrectEnableIfCheck>( "bugprone-incorrect-enable-if"); + CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>( + "bugprone-return-const-ref-from-parameter"); CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>( "bugprone-switch-missing-default-case"); CheckFactories.registerCheck<IncDecInConditionsCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 081ba67efe1538..2d303191f88650 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 + ReturnConstRefFromParameterCheck.cpp SuspiciousStringviewDataUsageCheck.cpp SwitchMissingDefaultCaseCheck.cpp IncDecInConditionsCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp new file mode 100644 index 00000000000000..87fc663231496e --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp @@ -0,0 +1,41 @@ +//===--- ReturnConstRefFromParameterCheck.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 "ReturnConstRefFromParameterCheck.h" +#include "../utils/Matchers.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/ASTMatchers/ASTMatchers.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +std::optional<TraversalKind> +ReturnConstRefFromParameterCheck::getCheckTraversalKind() const { + // Use 'AsIs' to make sure the return type is exactly the same as the + // parameter type. + return TK_AsIs; +} + +void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType( + hasCanonicalType(matchers::isReferenceToConst()))))))) + .bind("ret"), + this); +} + +void ReturnConstRefFromParameterCheck::check( + const MatchFinder::MatchResult &Result) { + const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret"); + diag(R->getRetValue()->getBeginLoc(), + "return const reference parameter cause potential use-after-free " + "when function accepts immediately constructed value."); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h new file mode 100644 index 00000000000000..96d02aa2563edf --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h @@ -0,0 +1,35 @@ +//===--- ReturnConstRefFromParameterCheck.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_RETURNCONSTREFFROMPARAMETERCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Detects the function which returns the const reference from parameter which +/// causes potential use after free if the caller uses xvalue as argument. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/return-const-ref-from-parameter.html +class ReturnConstRefFromParameterCheck : public ClangTidyCheck { +public: + ReturnConstRefFromParameterCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override; + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index a457e6fcae9462..3e5ad76bde3243 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -112,6 +112,12 @@ New checks Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP can be constructed outside itself and the derived class. +- New :doc:`bugprone-return-const-ref-from-parameter + <clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check. + + Detects the function which returns the const reference from parameter which + causes potential use after free if the caller uses xvalue as argument. + - New :doc:`bugprone-suspicious-stringview-data-usage <clang-tidy/checks/bugprone/suspicious-stringview-data-usage>` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst new file mode 100644 index 00000000000000..ec3fd439feb623 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst @@ -0,0 +1,30 @@ +.. title:: clang-tidy - bugprone-return-const-ref-from-parameter + +bugprone-return-const-ref-from-parameter +======================================== + +Detects the function which returns the const reference from parameter which +causes potential use after free if the caller uses xvalue as argument. + +In c++, const reference parameter can accept xvalue which will be destructed +after the call. When the function returns this parameter also as const reference, +then the returned reference can be used after the object it refers to has been +destroyed. + +Example +------- + +.. code-block:: c++ + + struct S { + int v; + S(int); + ~S(); + }; + + const S &fn(const S &a) { + return a; + } + + const S& s = fn(S{1}); + s.v; // use after free diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 8bc46acad56c84..5d9d487f75f9cb 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-posix-return <bugprone/posix-return>`, "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>` :doc:`bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch>`, "Yes" :doc:`bugprone-signal-handler <bugprone/signal-handler>`, :doc:`bugprone-signed-char-misuse <bugprone/signed-char-misuse>`, @@ -341,9 +342,9 @@ Clang-Tidy Checks :doc:`portability-std-allocator-const <portability/std-allocator-const>`, :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" :doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`, - :doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, + :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" @@ -529,12 +530,12 @@ Clang-Tidy Checks :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/return-const-ref-from-parameter.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp new file mode 100644 index 00000000000000..35e4476298b55f --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/return-const-ref-from-parameter.cpp @@ -0,0 +1,31 @@ +// RUN: %check_clang_tidy %s bugprone-return-const-ref-from-parameter %t + +using T = int; +using TConst = int const; +using TConstRef = int const&; + +namespace invalid { + +int const &f1(int const &a) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: return const reference parameter + +int const &f2(T const &a) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: return const reference parameter + +int const &f3(TConstRef a) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: return const reference parameter + +int const &f4(TConst &a) { return a; } +// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: return const reference parameter + +} // namespace invalid + +namespace valid { + +int const &f1(int &a) { return a; } + +int const &f2(int &&a) { return a; } + +int f1(int const &a) { return a; } + +} // namespace valid >From 401c775665159026a6b90158d21709948a1e7306 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Sun, 21 Apr 2024 11:55:22 +0800 Subject: [PATCH 2/3] fix doc --- .../bugprone/ReturnConstRefFromParameterCheck.cpp | 7 ------- .../bugprone/ReturnConstRefFromParameterCheck.h | 11 ++++++++--- .../bugprone/return-const-ref-from-parameter.rst | 9 +++++---- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp index 87fc663231496e..7670bf05b91192 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.cpp @@ -15,13 +15,6 @@ using namespace clang::ast_matchers; namespace clang::tidy::bugprone { -std::optional<TraversalKind> -ReturnConstRefFromParameterCheck::getCheckTraversalKind() const { - // Use 'AsIs' to make sure the return type is exactly the same as the - // parameter type. - return TK_AsIs; -} - void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher( returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType( diff --git a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h index 96d02aa2563edf..57e5732e8c0a98 100644 --- a/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/ReturnConstRefFromParameterCheck.h @@ -13,8 +13,9 @@ namespace clang::tidy::bugprone { -/// Detects the function which returns the const reference from parameter which -/// causes potential use after free if the caller uses xvalue as argument. +/// Detects the function which returns the const reference parameter as const +/// reference. This might causes potential use after free errors if the caller +/// uses xvalue as arguments. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/return-const-ref-from-parameter.html @@ -24,7 +25,11 @@ class ReturnConstRefFromParameterCheck : public ClangTidyCheck { : ClangTidyCheck(Name, Context) {} void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; - std::optional<TraversalKind> getCheckTraversalKind() const override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + // Use 'AsIs' to make sure the return type is exactly the same as the + // parameter type. + return TK_AsIs; + } bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst index ec3fd439feb623..f61f24ca1a263d 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/return-const-ref-from-parameter.rst @@ -3,11 +3,12 @@ bugprone-return-const-ref-from-parameter ======================================== -Detects the function which returns the const reference from parameter which -causes potential use after free if the caller uses xvalue as argument. +Detects the function which returns the const reference parameter as const +reference. This might causes potential use after free errors if the caller +uses xvalue as arguments. -In c++, const reference parameter can accept xvalue which will be destructed -after the call. When the function returns this parameter also as const reference, +In C++, const reference parameters can accept xvalues which will be destructed +after the call. When the function returns such a parameter also as const reference, then the returned reference can be used after the object it refers to has been destroyed. >From 322a81986f3337b610bc2c806a0aef72013faa75 Mon Sep 17 00:00:00 2001 From: Congcong Cai <congcongcai0...@163.com> Date: Sun, 21 Apr 2024 11:56:59 +0800 Subject: [PATCH 3/3] revert list --- clang-tools-extra/docs/clang-tidy/checks/list.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5d9d487f75f9cb..35633151b9f249 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -342,9 +342,9 @@ Clang-Tidy Checks :doc:`portability-std-allocator-const <portability/std-allocator-const>`, :doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes" :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-return-with-void-value <readability/avoid-return-with-void-value>`, :doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`, - :doc:`readability-braces-around-statements <readability/braces-around-statements>`, + :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" :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" @@ -530,12 +530,12 @@ Clang-Tidy Checks :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>`, + :doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" :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>`, + :doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes" :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>`, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits