Skip to content

Commit 91915f6

Browse files
committed
[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
1 parent b41179d commit 91915f6

File tree

8 files changed

+152
-4
lines changed

8 files changed

+152
-4
lines changed

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
#include "PosixReturnCheck.h"
5555
#include "RedundantBranchConditionCheck.h"
5656
#include "ReservedIdentifierCheck.h"
57+
#include "ReturnConstRefFromParameterCheck.h"
5758
#include "SharedPtrArrayMismatchCheck.h"
5859
#include "SignalHandlerCheck.h"
5960
#include "SignedCharMisuseCheck.h"
@@ -137,6 +138,8 @@ class BugproneModule : public ClangTidyModule {
137138
"bugprone-inaccurate-erase");
138139
CheckFactories.registerCheck<IncorrectEnableIfCheck>(
139140
"bugprone-incorrect-enable-if");
141+
CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
142+
"bugprone-return-const-ref-from-parameter");
140143
CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
141144
"bugprone-switch-missing-default-case");
142145
CheckFactories.registerCheck<IncDecInConditionsCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ add_clang_library(clangTidyBugproneModule
2626
ImplicitWideningOfMultiplicationResultCheck.cpp
2727
InaccurateEraseCheck.cpp
2828
IncorrectEnableIfCheck.cpp
29+
ReturnConstRefFromParameterCheck.cpp
2930
SuspiciousStringviewDataUsageCheck.cpp
3031
SwitchMissingDefaultCaseCheck.cpp
3132
IncDecInConditionsCheck.cpp
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
//===--- ReturnConstRefFromParameterCheck.cpp - clang-tidy ----------------===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#include "ReturnConstRefFromParameterCheck.h"
10+
#include "../utils/Matchers.h"
11+
#include "clang/ASTMatchers/ASTMatchFinder.h"
12+
#include "clang/ASTMatchers/ASTMatchers.h"
13+
14+
using namespace clang::ast_matchers;
15+
16+
namespace clang::tidy::bugprone {
17+
18+
std::optional<TraversalKind>
19+
ReturnConstRefFromParameterCheck::getCheckTraversalKind() const {
20+
// Use 'AsIs' to make sure the return type is exactly the same as the
21+
// parameter type.
22+
return TK_AsIs;
23+
}
24+
25+
void ReturnConstRefFromParameterCheck::registerMatchers(MatchFinder *Finder) {
26+
Finder->addMatcher(
27+
returnStmt(hasReturnValue(declRefExpr(to(parmVarDecl(hasType(
28+
hasCanonicalType(matchers::isReferenceToConst())))))))
29+
.bind("ret"),
30+
this);
31+
}
32+
33+
void ReturnConstRefFromParameterCheck::check(
34+
const MatchFinder::MatchResult &Result) {
35+
const auto *R = Result.Nodes.getNodeAs<ReturnStmt>("ret");
36+
diag(R->getRetValue()->getBeginLoc(),
37+
"return const reference parameter cause potential use-after-free "
38+
"when function accepts immediately constructed value.");
39+
}
40+
41+
} // namespace clang::tidy::bugprone
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
//===--- ReturnConstRefFromParameterCheck.h - clang-tidy --------*- C++ -*-===//
2+
//
3+
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
4+
// See https://llvm.org/LICENSE.txt for license information.
5+
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
6+
//
7+
//===----------------------------------------------------------------------===//
8+
9+
#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H
10+
#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H
11+
12+
#include "../ClangTidyCheck.h"
13+
14+
namespace clang::tidy::bugprone {
15+
16+
/// Detects the function which returns the const reference from parameter which
17+
/// causes potential use after free if the caller uses xvalue as argument.
18+
///
19+
/// For the user-facing documentation see:
20+
/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/return-const-ref-from-parameter.html
21+
class ReturnConstRefFromParameterCheck : public ClangTidyCheck {
22+
public:
23+
ReturnConstRefFromParameterCheck(StringRef Name, ClangTidyContext *Context)
24+
: ClangTidyCheck(Name, Context) {}
25+
void registerMatchers(ast_matchers::MatchFinder *Finder) override;
26+
void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
27+
std::optional<TraversalKind> getCheckTraversalKind() const override;
28+
bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
29+
return LangOpts.CPlusPlus;
30+
}
31+
};
32+
33+
} // namespace clang::tidy::bugprone
34+
35+
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_RETURNCONSTREFFROMPARAMETERCHECK_H

clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@ New checks
112112
Detects error-prone Curiously Recurring Template Pattern usage, when the CRTP
113113
can be constructed outside itself and the derived class.
114114

115+
- New :doc:`bugprone-return-const-ref-from-parameter
116+
<clang-tidy/checks/bugprone/return-const-ref-from-parameter>` check.
117+
118+
Detects the function which returns the const reference from parameter which
119+
causes potential use after free if the caller uses xvalue as argument.
120+
115121
- New :doc:`bugprone-suspicious-stringview-data-usage
116122
<clang-tidy/checks/bugprone/suspicious-stringview-data-usage>` check.
117123

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
.. title:: clang-tidy - bugprone-return-const-ref-from-parameter
2+
3+
bugprone-return-const-ref-from-parameter
4+
========================================
5+
6+
Detects the function which returns the const reference from parameter which
7+
causes potential use after free if the caller uses xvalue as argument.
8+
9+
In c++, const reference parameter can accept xvalue which will be destructed
10+
after the call. When the function returns this parameter also as const reference,
11+
then the returned reference can be used after the object it refers to has been
12+
destroyed.
13+
14+
Example
15+
-------
16+
17+
.. code-block:: c++
18+
19+
struct S {
20+
int v;
21+
S(int);
22+
~S();
23+
};
24+
25+
const S &fn(const S &a) {
26+
return a;
27+
}
28+
29+
const S& s = fn(S{1});
30+
s.v; // use after free

clang-tools-extra/docs/clang-tidy/checks/list.rst

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ Clang-Tidy Checks
120120
:doc:`bugprone-posix-return <bugprone/posix-return>`, "Yes"
121121
:doc:`bugprone-redundant-branch-condition <bugprone/redundant-branch-condition>`, "Yes"
122122
:doc:`bugprone-reserved-identifier <bugprone/reserved-identifier>`, "Yes"
123+
:doc:`bugprone-return-const-ref-from-parameter <bugprone/return-const-ref-from-parameter>`
123124
:doc:`bugprone-shared-ptr-array-mismatch <bugprone/shared-ptr-array-mismatch>`, "Yes"
124125
:doc:`bugprone-signal-handler <bugprone/signal-handler>`,
125126
:doc:`bugprone-signed-char-misuse <bugprone/signed-char-misuse>`,
@@ -341,9 +342,9 @@ Clang-Tidy Checks
341342
:doc:`portability-std-allocator-const <portability/std-allocator-const>`,
342343
:doc:`readability-avoid-const-params-in-decls <readability/avoid-const-params-in-decls>`, "Yes"
343344
:doc:`readability-avoid-nested-conditional-operator <readability/avoid-nested-conditional-operator>`,
344-
:doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`,
345+
:doc:`readability-avoid-return-with-void-value <readability/avoid-return-with-void-value>`, "Yes"
345346
:doc:`readability-avoid-unconditional-preprocessor-if <readability/avoid-unconditional-preprocessor-if>`,
346-
:doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
347+
:doc:`readability-braces-around-statements <readability/braces-around-statements>`,
347348
:doc:`readability-const-return-type <readability/const-return-type>`, "Yes"
348349
:doc:`readability-container-contains <readability/container-contains>`, "Yes"
349350
:doc:`readability-container-data-pointer <readability/container-data-pointer>`, "Yes"
@@ -529,12 +530,12 @@ Clang-Tidy Checks
529530
: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>`,
530531
:doc:`cppcoreguidelines-use-default-member-init <cppcoreguidelines/use-default-member-init>`, :doc:`modernize-use-default-member-init <modernize/use-default-member-init>`, "Yes"
531532
:doc:`fuchsia-header-anon-namespaces <fuchsia/header-anon-namespaces>`, :doc:`google-build-namespaces <google/build-namespaces>`,
532-
:doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
533+
:doc:`google-readability-braces-around-statements <google/readability-braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`,
533534
:doc:`google-readability-function-size <google/readability-function-size>`, :doc:`readability-function-size <readability/function-size>`,
534535
:doc:`google-readability-namespace-comments <google/readability-namespace-comments>`, :doc:`llvm-namespace-comment <llvm/namespace-comment>`,
535536
:doc:`hicpp-avoid-c-arrays <hicpp/avoid-c-arrays>`, :doc:`modernize-avoid-c-arrays <modernize/avoid-c-arrays>`,
536537
:doc:`hicpp-avoid-goto <hicpp/avoid-goto>`, :doc:`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto>`,
537-
:doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`, "Yes"
538+
:doc:`hicpp-braces-around-statements <hicpp/braces-around-statements>`, :doc:`readability-braces-around-statements <readability/braces-around-statements>`,
538539
:doc:`hicpp-deprecated-headers <hicpp/deprecated-headers>`, :doc:`modernize-deprecated-headers <modernize/deprecated-headers>`, "Yes"
539540
:doc:`hicpp-explicit-conversions <hicpp/explicit-conversions>`, :doc:`google-explicit-constructor <google/explicit-constructor>`, "Yes"
540541
:doc:`hicpp-function-size <hicpp/function-size>`, :doc:`readability-function-size <readability/function-size>`,
Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// RUN: %check_clang_tidy %s bugprone-return-const-ref-from-parameter %t
2+
3+
using T = int;
4+
using TConst = int const;
5+
using TConstRef = int const&;
6+
7+
namespace invalid {
8+
9+
int const &f1(int const &a) { return a; }
10+
// CHECK-MESSAGES: :[[@LINE-1]]:38: warning: return const reference parameter
11+
12+
int const &f2(T const &a) { return a; }
13+
// CHECK-MESSAGES: :[[@LINE-1]]:36: warning: return const reference parameter
14+
15+
int const &f3(TConstRef a) { return a; }
16+
// CHECK-MESSAGES: :[[@LINE-1]]:37: warning: return const reference parameter
17+
18+
int const &f4(TConst &a) { return a; }
19+
// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: return const reference parameter
20+
21+
} // namespace invalid
22+
23+
namespace valid {
24+
25+
int const &f1(int &a) { return a; }
26+
27+
int const &f2(int &&a) { return a; }
28+
29+
int f1(int const &a) { return a; }
30+
31+
} // namespace valid

0 commit comments

Comments
 (0)