[clang-tools-extra] 17d403f - [clang-tidy] Finish cppcoreguidelines-avoid-capturing-lambda-coroutines check
Piotr Zegar via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 10 06:07:09 PST 2023
Author: Piotr Zegar
Date: 2023-03-10T14:06:50Z
New Revision: 17d403f6644337e3e099e6dcb7b057fce11e65a5
URL: https://github.com/llvm/llvm-project/commit/17d403f6644337e3e099e6dcb7b057fce11e65a5
DIFF: https://github.com/llvm/llvm-project/commit/17d403f6644337e3e099e6dcb7b057fce11e65a5.diff
LOG: [clang-tidy] Finish cppcoreguidelines-avoid-capturing-lambda-coroutines check
This commit implements check for CP.51 C++ Core Guidelines
Depends on D137514
Reviewed By: ChuanqiXu
Differential Revision: https://reviews.llvm.org/D145720
Added:
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h
Modified:
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
index de9c9eaa7f407..3c99831f9d640 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.cpp
@@ -12,34 +12,34 @@
using namespace clang::ast_matchers;
-namespace clang {
-namespace tidy {
-namespace cppcoreguidelines {
+namespace clang::tidy::cppcoreguidelines {
+
+namespace {
+AST_MATCHER(LambdaExpr, hasCoroutineBody) {
+ const Stmt *Body = Node.getBody();
+ return Body != nullptr && CoroutineBodyStmt::classof(Body);
+}
+
+AST_MATCHER(LambdaExpr, hasCaptures) { return Node.capture_size() != 0U; }
+} // namespace
void AvoidCapturingLambdaCoroutinesCheck::registerMatchers(
MatchFinder *Finder) {
- Finder->addMatcher(lambdaExpr().bind("lambda"), this);
+ Finder->addMatcher(
+ lambdaExpr(hasCaptures(), hasCoroutineBody()).bind("lambda"), this);
+}
+
+bool AvoidCapturingLambdaCoroutinesCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus20;
}
void AvoidCapturingLambdaCoroutinesCheck::check(
const MatchFinder::MatchResult &Result) {
- const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
- if (!Lambda) {
- return;
- }
-
- const auto *Body = dyn_cast<CoroutineBodyStmt>(Lambda->getBody());
- if (!Body) {
- return;
- }
-
- if (Lambda->captures().empty()) {
- return;
- }
-
- diag(Lambda->getBeginLoc(), "found capturing coroutine lambda");
+ const auto *MatchedLambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+ diag(MatchedLambda->getExprLoc(),
+ "coroutine lambda may cause use-after-free, avoid captures or ensure "
+ "lambda closure object has guaranteed lifetime");
}
-} // namespace cppcoreguidelines
-} // namespace tidy
-} // namespace clang
+} // namespace clang::tidy::cppcoreguidelines
diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
index 63895f3354414..b32e2662b5fba 100644
--- a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
+++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCapturingLambdaCoroutinesCheck.h
@@ -11,26 +11,23 @@
#include "../ClangTidyCheck.h"
-namespace clang {
-namespace tidy {
-namespace cppcoreguidelines {
+namespace clang::tidy::cppcoreguidelines {
-/// The normal usage of captures in lambdas are problematic when the lambda is a
-/// coroutine because the captures are destroyed after the first suspension
-/// point. Using the captures after this point is a use-after-free issue.
+/// Flags C++20 coroutine lambdas with non-empty capture lists that may cause
+/// use-after-free errors and suggests avoiding captures or ensuring the lambda
+/// closure object has a guaranteed lifetime.
///
/// For the user-facing documentation see:
-/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines-avoid-capturing-lambda-coroutines.html
+/// https://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.html
class AvoidCapturingLambdaCoroutinesCheck : public ClangTidyCheck {
public:
AvoidCapturingLambdaCoroutinesCheck(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;
};
-} // namespace cppcoreguidelines
-} // namespace tidy
-} // namespace clang
+} // namespace clang::tidy::cppcoreguidelines
#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDCAPTURINGLAMBDACOROUTINESCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 64908944913fc..bda6168f3aacb 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -123,9 +123,9 @@ New checks
- New :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines
<clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines>` check.
- Adds check for cpp core guideline: "CP.51: Do not use capturing lambdas that
- are coroutines."
-
+ Flags C++20 coroutine lambdas with non-empty capture lists that may cause
+ use-after-free errors and suggests avoiding captures or ensuring the lambda
+ closure object has a guaranteed lifetime.
New check aliases
^^^^^^^^^^^^^^^^^
diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
index c199152edeac3..74ec5045a6bbf 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capturing-lambda-coroutines.rst
@@ -3,29 +3,52 @@
cppcoreguidelines-avoid-capturing-lambda-coroutines
===================================================
-Warns if a capturing lambda is a coroutine. For example:
+Flags C++20 coroutine lambdas with non-empty capture lists that may cause
+use-after-free errors and suggests avoiding captures or ensuring the lambda
+closure object has a guaranteed lifetime.
+
+This check implements `CP.51
+<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rcoro-capture>`_
+from the C++ Core Guidelines.
+
+Using coroutine lambdas with non-empty capture lists can be risky, as capturing
+variables can lead to accessing freed memory after the first suspension point.
+This issue can occur even with refcounted smart pointers and copyable types.
+When a lambda expression creates a coroutine, it results in a closure object
+with storage, which is often on the stack and will eventually go out of scope.
+When the closure object goes out of scope, its captures also go out of scope.
+While normal lambdas finish executing before this happens, coroutine lambdas may
+resume from suspension after the closure object has been destructed, resulting
+in use-after-free memory access for all captures.
+
+Consider the following example:
.. code-block:: c++
- int c;
-
- [c] () -> task { co_return; };
- [&] () -> task { int y = c; co_return; };
- [=] () -> task { int y = c; co_return; };
-
- struct s {
- void m() {
- [this] () -> task { co_return; };
- }
- };
-
-All of the cases above will trigger the warning. However, implicit captures
-do not trigger the warning unless the body of the lambda uses the capture.
-For example, the following do not trigger the warning.
-
-.. code-block:: c++
-
- int c;
-
- [&] () -> task { co_return; };
- [=] () -> task { co_return; };
+ int value = get_value();
+ std::shared_ptr<Foo> sharedFoo = get_foo();
+ {
+ const auto lambda = [value, sharedFoo]() -> std::future<void>
+ {
+ co_await something();
+ // "sharedFoo" and "value" have already been destroyed
+ // the "shared" pointer didn't accomplish anything
+ };
+ lambda();
+ } // the lambda closure object has now gone out of scope
+
+In this example, the lambda object is defined with two captures: value and
+``sharedFoo``. When ``lambda()`` is called, the lambda object is created on the
+stack, and the captures are copied into the closure object. When the coroutine
+is suspended, the lambda object goes out of scope, and the closure object is
+destroyed. When the coroutine is resumed, the captured variables may have been
+destroyed, resulting in use-after-free bugs.
+
+In conclusion, the use of coroutine lambdas with non-empty capture lists can
+lead to use-after-free errors when resuming the coroutine after the closure
+object has been destroyed. This check helps prevent such errors by flagging
+C++20 coroutine lambdas with non-empty capture lists and suggesting avoiding
+captures or ensuring the lambda closure object has a guaranteed lifetime.
+
+Following these guidelines can help ensure the safe and reliable use of
+coroutine lambdas in C++ code.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 0eb15a9d382e0..6385b754d3b7d 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -180,7 +180,7 @@ Clang-Tidy Checks
`concurrency-mt-unsafe <concurrency/mt-unsafe.html>`_,
`concurrency-thread-canceltype-asynchronous <concurrency/thread-canceltype-asynchronous.html>`_,
`cppcoreguidelines-avoid-capture-default-when-capturing-this <cppcoreguidelines/avoid-capture-default-when-capturing-this.html>`_,
- `cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines.html>`_, "Yes"
+ `cppcoreguidelines-avoid-capturing-lambda-coroutines <cppcoreguidelines/avoid-capturing-lambda-coroutines.html>`_,
`cppcoreguidelines-avoid-const-or-ref-data-members <cppcoreguidelines/avoid-const-or-ref-data-members.html>`_,
`cppcoreguidelines-avoid-do-while <cppcoreguidelines/avoid-do-while.html>`_,
`cppcoreguidelines-avoid-goto <cppcoreguidelines/avoid-goto.html>`_,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h
new file mode 100644
index 0000000000000..ed4373394351b
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/Inputs/system/coroutines.h
@@ -0,0 +1,32 @@
+#pragma once
+
+namespace std {
+
+template <typename ret_t, typename... args_t>
+struct coroutine_traits {
+ using promise_type = typename ret_t::promise_type;
+};
+
+template <class promise_t>
+struct coroutine_handle {
+ static constexpr coroutine_handle from_address(void *addr) noexcept { return {}; };
+};
+
+} // namespace std
+
+struct never_suspend {
+ bool await_ready() noexcept { return false; }
+ template <typename coro_t>
+ void await_suspend(coro_t handle) noexcept {}
+ void await_resume() noexcept {}
+};
+
+struct task {
+ struct promise_type {
+ task get_return_object() noexcept { return {}; }
+ never_suspend initial_suspend() noexcept { return {}; }
+ never_suspend final_suspend() noexcept { return {}; }
+ void return_void() {}
+ void unhandled_exception() {}
+ };
+};
diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
index 9a8a88e5d0283..6670f4a5420be 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capturing-lambda-coroutines.cpp
@@ -1,5 +1,4 @@
-// RUN: %check_clang_tidy -std=c++20-or-later %s cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- \
-// RUN: -isystem %S/../readability/Inputs/identifier-naming/system
+// RUN: %check_clang_tidy -std=c++20-or-later %s cppcoreguidelines-avoid-capturing-lambda-coroutines %t -- -- -isystem %S/Inputs/system
#include <coroutines.h>
@@ -7,23 +6,23 @@ void Caught() {
int v;
[&] () -> task { int y = v; co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
[=] () -> task { int y = v; co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
[v] () -> task { co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
[&v] () -> task { co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
[y=v] () -> task { co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
[y{v}] () -> task { co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:5: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:5: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
}
struct S {
void m() {
[this] () -> task { co_return; };
- // CHECK-MESSAGES: [[@LINE-1]]:9: warning: found capturing coroutine lambda [cppcoreguidelines-avoid-capturing-lambda-coroutines]
+ // CHECK-MESSAGES: [[@LINE-1]]:9: warning: coroutine lambda may cause use-after-free, avoid captures or ensure lambda closure object has guaranteed lifetime [cppcoreguidelines-avoid-capturing-lambda-coroutines]
}
};
@@ -32,4 +31,6 @@ void Safe() {
[] () -> task { co_return; };
[&] () -> task { co_return; };
[=] () -> task { co_return; };
+
+ [&v]{++v;}();
}
More information about the cfe-commits
mailing list