[clang-tools-extra] dfa0db7 - Warn pointer captured in async block

Aaron Ballman via cfe-commits cfe-commits at lists.llvm.org
Tue Jul 7 10:31:24 PDT 2020


Author: Ellis Hoag
Date: 2020-07-07T13:31:14-04:00
New Revision: dfa0db79d0e37d5cf24a63d1e2b7ba5f40617574

URL: https://github.com/llvm/llvm-project/commit/dfa0db79d0e37d5cf24a63d1e2b7ba5f40617574
DIFF: https://github.com/llvm/llvm-project/commit/dfa0db79d0e37d5cf24a63d1e2b7ba5f40617574.diff

LOG: Warn pointer captured in async block

The block arguments in dispatch_async() and dispatch_after() are
guaranteed to escape. If those blocks capture any pointers with the
noescape attribute then it is an error.

Added: 
    clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp
    clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
    clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
    clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m

Modified: 
    clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
    clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
index d010c3ce7e52..3f735a8484d8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -34,6 +34,7 @@
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "NoEscapeCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
@@ -120,6 +121,7 @@ class BugproneModule : public ClangTidyModule {
         "bugprone-multiple-statement-macro");
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
+    CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
     CheckFactories.registerCheck<NotNullTerminatedResultCheck>(
         "bugprone-not-null-terminated-result");
     CheckFactories.registerCheck<ParentVirtualCallCheck>(

diff  --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
index 6cc60ff0d7bc..6e7a94928a5a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -29,6 +29,7 @@ add_clang_library(clangTidyBugproneModule
   MisplacedWideningCastCheck.cpp
   MoveForwardingReferenceCheck.cpp
   MultipleStatementMacroCheck.cpp
+  NoEscapeCheck.cpp
   NotNullTerminatedResultCheck.cpp
   ParentVirtualCallCheck.cpp
   PosixReturnCheck.cpp

diff  --git a/clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp
new file mode 100644
index 000000000000..654e80d9a9c6
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.cpp
@@ -0,0 +1,51 @@
+//===--- NoEscapeCheck.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 "NoEscapeCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+void NoEscapeCheck::registerMatchers(MatchFinder *Finder) {
+  Finder->addMatcher(callExpr(callee(functionDecl(hasName("::dispatch_async"))),
+                              argumentCountIs(2),
+                              hasArgument(1, blockExpr().bind("arg-block"))),
+                     this);
+  Finder->addMatcher(callExpr(callee(functionDecl(hasName("::dispatch_after"))),
+                              argumentCountIs(3),
+                              hasArgument(2, blockExpr().bind("arg-block"))),
+                     this);
+}
+
+void NoEscapeCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedEscapingBlock =
+      Result.Nodes.getNodeAs<BlockExpr>("arg-block");
+  const BlockDecl *EscapingBlockDecl = MatchedEscapingBlock->getBlockDecl();
+  for (const BlockDecl::Capture &CapturedVar : EscapingBlockDecl->captures()) {
+    const VarDecl *Var = CapturedVar.getVariable();
+    if (Var && Var->hasAttr<NoEscapeAttr>()) {
+      // FIXME: Add a method to get the location of the use of a CapturedVar so
+      // that we can diagnose the use of the pointer instead of the block.
+      diag(MatchedEscapingBlock->getBeginLoc(),
+           "pointer %0 with attribute 'noescape' is captured by an "
+           "asynchronously-executed block")
+          << Var;
+      diag(Var->getBeginLoc(), "the 'noescape' attribute is declared here.",
+           DiagnosticIDs::Note);
+    }
+  }
+}
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang

diff  --git a/clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h b/clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
new file mode 100644
index 000000000000..126c37c9df0a
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/bugprone/NoEscapeCheck.h
@@ -0,0 +1,39 @@
+//===--- NoEscapeCheck.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_NOESCAPECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOESCAPECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace bugprone {
+
+/// Block arguments in `dispatch_async()` and `dispatch_after()` are guaranteed
+/// to escape. If those blocks capture any pointers with the `noescape`
+/// attribute, then we warn the user of their error.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone-no-escape.html
+class NoEscapeCheck : public ClangTidyCheck {
+public:
+  NoEscapeCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+    return LangOpts.Blocks;
+  }
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace bugprone
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NOESCAPECHECK_H

diff  --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index c66d5eb6069e..c08fd45c2f96 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -94,6 +94,12 @@ New checks
   result of a memory allocation function (``malloc()``, ``calloc()``,
   ``realloc()``, ``alloca()``) instead of its argument.
 
+- New :doc:`bugprone-no-escape
+  <clang-tidy/checks/bugprone-no-escape>` check.
+
+  Finds pointers with the ``noescape`` attribute that are captured by an
+  asynchronously-executed block.
+
 - New :doc:`bugprone-spuriously-wake-up-functions
   <clang-tidy/checks/bugprone-spuriously-wake-up-functions>` check.
 
@@ -206,14 +212,14 @@ Changes in existing checks
   Now checks ``std::basic_string_view`` by default.
 
 - Improved :doc:`readability-else-after-return
-  <clang-tidy/checks/readability-else-after-return>` check now supports a 
+  <clang-tidy/checks/readability-else-after-return>` check now supports a
   `WarnOnConditionVariables` option to control whether to refactor condition
   variables where possible.
 
 - Improved :doc:`readability-identifier-naming
   <clang-tidy/checks/readability-identifier-naming>` check.
 
-  Now able to rename member references in class template definitions with 
+  Now able to rename member references in class template definitions with
   explicit access.
 
 - Improved :doc:`readability-qualified-auto

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
new file mode 100644
index 000000000000..ea137b9679cc
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-no-escape.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - bugprone-no-escape
+
+bugprone-no-escape
+==================
+
+Finds pointers with the ``noescape`` attribute that are captured by an
+asynchronously-executed block. The block arguments in ``dispatch_async()`` and
+``dispatch_after()`` are guaranteed to escape, so it is an error if a pointer with the
+``noescape`` attribute is captured by one of these blocks.
+
+The following is an example of an invalid use of the ``noescape`` attribute.
+
+  .. code-block:: objc
+    void foo(__attribute__((noescape)) int *p) {
+      dispatch_async(queue, ^{
+        *p = 123;
+      });
+    });

diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 7356c585d20c..78fed1a8ed9c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -70,6 +70,7 @@ Clang-Tidy Checks
    `bugprone-misplaced-widening-cast <bugprone-misplaced-widening-cast.html>`_,
    `bugprone-move-forwarding-reference <bugprone-move-forwarding-reference.html>`_, "Yes"
    `bugprone-multiple-statement-macro <bugprone-multiple-statement-macro.html>`_,
+   `bugprone-no-escape <bugprone-no-escape.html>`_, "Yes"
    `bugprone-not-null-terminated-result <bugprone-not-null-terminated-result.html>`_, "Yes"
    `bugprone-parent-virtual-call <bugprone-parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone-posix-return.html>`_, "Yes"

diff  --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m b/clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m
new file mode 100644
index 000000000000..11dba3345fa1
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone-no-escape.m
@@ -0,0 +1,28 @@
+// RUN: %check_clang_tidy %s bugprone-no-escape %t
+// RUN: %check_clang_tidy %s -assume-filename=bugprone-no-escape.c bugprone-no-escape %t -- -- -fblocks
+
+typedef struct dispatch_queue_s *dispatch_queue_t;
+typedef struct dispatch_time_s *dispatch_time_t;
+typedef void (^dispatch_block_t)(void);
+void dispatch_async(dispatch_queue_t queue, dispatch_block_t block);
+void dispatch_after(dispatch_time_t when, dispatch_queue_t queue, dispatch_block_t block);
+
+extern dispatch_queue_t queue;
+
+void test_noescape_attribute(__attribute__((noescape)) int *p, int *q) {
+  dispatch_async(queue, ^{
+    *p = 123;
+    // CHECK-MESSAGES: :[[@LINE-2]]:25: warning: pointer 'p' with attribute 'noescape' is captured by an asynchronously-executed block [bugprone-no-escape]
+    // CHECK-MESSAGES: :[[@LINE-4]]:30: note: the 'noescape' attribute is declared here.
+  });
+
+  dispatch_after(456, queue, ^{
+    *p = 789;
+    // CHECK-MESSAGES: :[[@LINE-2]]:30: warning: pointer 'p' with attribute 'noescape' is captured by an asynchronously-executed block [bugprone-no-escape]
+  });
+
+  dispatch_async(queue, ^{
+    *q = 0;
+    // CHECK-MESSAGES-NOT: :[[@LINE-2]]:25: warning: pointer 'q' with attribute 'noescape' is captured by an asynchronously-executed block
+  });
+}


        


More information about the cfe-commits mailing list