Feature/labmda uaf

[segoon]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang-tidy

@llvm/pr-subscribers-clang-tools-extra

Author: Vasiliy Kulikov (segoon)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/145307.diff

8 Files Affected:

• (modified) clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp (+3)
• (modified) clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt (+1)
• (added) clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.cpp (+58)
• (added) clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.h (+33)
• (modified) clang-tools-extra/docs/ReleaseNotes.rst (+5)
• (added) clang-tools-extra/docs/clang-tidy/checks/bugprone/taxi-async-use-after-free.rst (+6)
• (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
• (added) clang-tools-extra/test/clang-tidy/checkers/bugprone/taxi-async-use-after-free.cpp (+53)

diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index b780a85bdf3fe..e22741e66580f 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -83,6 +83,7 @@
 #include "SwappedArgumentsCheck.h"
 #include "SwitchMissingDefaultCaseCheck.h"
 #include "TaggedUnionMemberCountCheck.h"
+#include "TaxiAsyncUseAfterFreeCheck.h"
 #include "TerminatingContinueCheck.h"
 #include "ThrowKeywordMissingCheck.h"
 #include "TooSmallLoopVariableCheck.h"
@@ -151,6 +152,8 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-incorrect-enable-if");
     CheckFactories.registerCheck<IncorrectEnableSharedFromThisCheck>(
         "bugprone-incorrect-enable-shared-from-this");
+    CheckFactories.registerCheck<TaxiAsyncUseAfterFreeCheck>(
+        "bugprone-taxi-async-use-after-free");
     CheckFactories.registerCheck<UnintendedCharOstreamOutputCheck>(
         "bugprone-unintended-char-ostream-output");
     CheckFactories.registerCheck<ReturnConstRefFromParameterCheck>(
diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index e310ea9c94543..b39177320b6ce 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -30,6 +30,7 @@ add_clang_library(clangTidyBugproneModule STATIC
   InaccurateEraseCheck.cpp
   IncorrectEnableIfCheck.cpp
   IncorrectEnableSharedFromThisCheck.cpp
+  TaxiAsyncUseAfterFreeCheck.cpp
   UnintendedCharOstreamOutputCheck.cpp
   ReturnConstRefFromParameterCheck.cpp
   SuspiciousStringviewDataUsageCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.cpp
new file mode 100644
index 0000000000000..c044cf7b56f72
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.cpp
@@ -0,0 +1,58 @@
+//===--- TaxiAsyncUseAfterFreeCheck.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 "TaxiAsyncUseAfterFreeCheck.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void TaxiAsyncUseAfterFreeCheck::registerMatchers(MatchFinder* Finder) {
+  auto hasAsyncName = hasAnyName(
+      "Async", "AsyncNoSpan", "SharedAsyncNoSpan", "CriticalAsyncNoSpan",
+      "SharedCriticalAsyncNoSpan", "CriticalAsync", "SharedCriticalAsync");
+
+  Finder->addMatcher(
+      lambdaExpr(
+          hasParent(materializeTemporaryExpr(hasParent(callExpr(
+              hasParent(cxxMemberCallExpr(
+
+                  callee(cxxMethodDecl(hasName("push_back"))),
+
+                  on(declRefExpr(hasDeclaration(varDecl().bind("tasks")))))),
+
+              callee(functionDecl(hasAsyncName)))))))
+          .bind("lambda"),
+      this);
+}
+
+void TaxiAsyncUseAfterFreeCheck::check(const MatchFinder::MatchResult& Result) {
+  const auto* MatchedLambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+  const auto* MatchedTasks = Result.Nodes.getNodeAs<VarDecl>("tasks");
+  const SourceLocation TasksLocation = MatchedTasks->getLocation();
+
+  for (const auto& Capture : MatchedLambda->captures()) {
+    if (!Capture.capturesVariable() || Capture.getCaptureKind() != LCK_ByRef)
+      continue;
+
+    const ValueDecl* CapturedVarDecl = Capture.getCapturedVar();
+    if (CapturedVarDecl->getType().getCanonicalType()->isLValueReferenceType() ||
+        CapturedVarDecl->getType().getCanonicalType()->isRValueReferenceType()) {
+      continue;
+    }
+
+    if (CapturedVarDecl->getLocation() >= TasksLocation) {
+      diag(Capture.getLocation(), "captured here");
+      diag(CapturedVarDecl->getLocation(), "variable can be used after free");
+      diag(TasksLocation, "std::vector<Task> can die after variable");
+    }
+  }
+}
+
+}  // namespace clang::tidy::bugprone
diff --git a/clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.h b/clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.h
new file mode 100644
index 0000000000000..7e56e6a2b3c23
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/TaxiAsyncUseAfterFreeCheck.h
@@ -0,0 +1,33 @@
+//===--- TaxiAsyncUseAfterFreeCheck.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_TAXIASYNCUSEAFTERFREECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_TAXIASYNCUSEAFTERFREECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Use-after-free for engine::Async.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/taxi-async-use-after-free.html
+class TaxiAsyncUseAfterFreeCheck : public ClangTidyCheck {
+public:
+  TaxiAsyncUseAfterFreeCheck(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_TAXIASYNCUSEAFTERFREECHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 579fca54924d5..ea1081e603b72 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -124,6 +124,11 @@ New checks
   pointer and store it as class members without handle the copy and move
   constructors and the assignments.
 
+- New :doc:`bugprone-taxi-async-use-after-free
+  <clang-tidy/checks/bugprone/taxi-async-use-after-free>` check.
+
+  Use-after-free for engine::Async.
+
 - New :doc:`bugprone-unintended-char-ostream-output
   <clang-tidy/checks/bugprone/unintended-char-ostream-output>` check.
 
diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/taxi-async-use-after-free.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/taxi-async-use-after-free.rst
new file mode 100644
index 0000000000000..bff4f8dd974b3
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/taxi-async-use-after-free.rst
@@ -0,0 +1,6 @@
+.. title:: clang-tidy - bugprone-taxi-async-use-after-free
+
+bugprone-taxi-async-use-after-free
+==================================
+
+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 18f1467285fab..c098ee3d5efa6 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -151,6 +151,7 @@ Clang-Tidy Checks
    :doc:`bugprone-swapped-arguments <bugprone/swapped-arguments>`, "Yes"
    :doc:`bugprone-switch-missing-default-case <bugprone/switch-missing-default-case>`,
    :doc:`bugprone-tagged-union-member-count <bugprone/tagged-union-member-count>`,
+   :doc:`bugprone-taxi-async-use-after-free <bugprone/taxi-async-use-after-free>`, "Yes"
    :doc:`bugprone-terminating-continue <bugprone/terminating-continue>`, "Yes"
    :doc:`bugprone-throw-keyword-missing <bugprone/throw-keyword-missing>`,
    :doc:`bugprone-too-small-loop-variable <bugprone/too-small-loop-variable>`,
diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/taxi-async-use-after-free.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/taxi-async-use-after-free.cpp
new file mode 100644
index 0000000000000..f8d4eee0411cb
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/taxi-async-use-after-free.cpp
@@ -0,0 +1,53 @@
+// RUN: %check_clang_tidy %s bugprone-taxi-async-use-after-free %t
+
+namespace std {
+
+template<typename T>
+class vector {
+public:
+  void push_back(T) {}
+};
+
+}
+
+namespace engine {
+
+template <typename Function, typename... Args>
+int Async(Function&& f, Args&&... args) {
+  return 1;
+}
+
+}
+
+void f_ok() {
+  int x = 1;
+  std::vector<int> v;
+
+  v.push_back(engine::Async([&x]{ x = 2;}));
+}
+
+void f_use_after_free() {
+  std::vector<int> v;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: std::vector<Task> can die after variable [bugprone-taxi-async-use-after-free]
+  int x = 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable can be used after free [bugprone-taxi-async-use-after-free]
+
+  v.push_back(engine::Async([&x]{ x = 2;}));
+  // CHECK-MESSAGES: :[[@LINE-1]]:31: warning: captured here [bugprone-taxi-async-use-after-free]
+}
+
+void f_ref() {
+  int xx = 1;
+  std::vector<int> v;
+  int &x = x;
+
+  v.push_back(engine::Async([&x]{ x = 2;}));
+}
+
+void f_ref_ref() {
+  int xx = 1;
+  std::vector<int> v;
+  int &&x = static_cast<int&&>(xx);
+
+  v.push_back(engine::Async([&x]{ x = 2;}));
+}

[vbvictor]

Is this unfinished work? I'd suggest to covert this to draft.

[EugeneZelenko]

Suggested change

[EugeneZelenko]

Suggested change

[EugeneZelenko]

Suggested change

[EugeneZelenko]

Please make it more human-readable.

[EugeneZelenko]

Missing documentation.

[vbvictor]

I suppose this check is specific to this https://github.com/userver-framework/userver framework, so we can't accept it in clang-tidy in current format.

[segoon]

Sorry, this PR was not indended to be merged as is =( sorry for the noise