[clang-tidy] New check to warn when writing to a dispatch_once_t variable.

[mwyman]

Summary

Libdispatch documentation specifies that dispatch_once_ts must never have been non-zero, and assigning to them violates this.

Currently adding this to the misc module. Per discussion in https://reviews.llvm.org/D67567, perhaps this should be moved to a new, more specific module.

[Eugene.Zelenko]

clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-assignment.rst

9

Does library documentation contain this recommendation? If so, will be good idea to add link. Same for other check.

[stephanemoore]

clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-assignment.rst

9

I don't recall if there is explicit GCD documentation anywhere. The closest thing I could find was a post from Greg Parker: https://stackoverflow.com/a/19845164.

I think the general issue is that GCD performs atomic operations internally on dispatch_once_t memory. A recent snapshot of a version of GCD suggests that C11 atomic operations are used but the general issue is that GCD does not publicly declare how it internally guarantees atomicity while managing dispatch_once_t state. The safest way to make mutations to dispatch_once_t memory is by using the same atomic operations library that the particular version of GCD loaded into your process uses (in theory this can vary depending on a number of factors including the system version). Given that this information is not exposed and it seems unlikely, if not unreasonable, to be exposed, the safest approach is presumably to to let GCD handle all mutations.

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

 set(LLVM_LINK_COMPONENTS support)
 
 add_clang_library(clangTidyMiscModule
   DefinitionsInHeadersCheck.cpp
+  DispatchOnceAssignmentCheck.cpp
   MiscTidyModule.cpp
   MisplacedConstCheck.cpp
   NewDeleteOverloadsCheck.cpp
   NonCopyableObjects.cpp
   NonPrivateMemberVariablesInClassesCheck.cpp
   RedundantExpressionCheck.cpp
   StaticAssertCheck.cpp
   ThrowByValueCatchByReferenceCheck.cpp

clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.h

+//===--- DispatchOnceAssignmentCheck.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_MISC_DISPATCHONCEASSIGNMENTCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DISPATCHONCEASSIGNMENTCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds explicit assignment to variables of type dispatch_once_t, which should
+/// not be assigned values directly in application code.
+///
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-dispatch-once-assignment.html
+class DispatchOnceAssignmentCheck : public ClangTidyCheck {
+public:
+  DispatchOnceAssignmentCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DISPATCHONCEASSIGNMENTCHECK_H

clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.cpp

+//===--- DispatchOnceAssignmentCheck.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 "DispatchOnceAssignmentCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Expr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void DispatchOnceAssignmentCheck::registerMatchers(MatchFinder *Finder) {
+  // Find assignments where the LHS has type dispatch_once_t.
+  Finder->addMatcher(
+      binaryOperator(isAssignmentOperator(),
+                     hasLHS(hasType(asString("dispatch_once_t"))))
+          .bind("assignment"),
+      this);
+}
+
+void DispatchOnceAssignmentCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  if (const auto *Op = Result.Nodes.getNodeAs<BinaryOperator>("assignment")) {
+    diag(Op->getExprLoc(),
+         "do not assign to dispatch_once_t variables");
+  }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang

clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp

 //===--- MiscTidyModule.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 "../ClangTidy.h"
 #include "../ClangTidyModule.h"
 #include "../ClangTidyModuleRegistry.h"
 #include "DefinitionsInHeadersCheck.h"
+#include "DispatchOnceAssignmentCheck.h"
 #include "MisplacedConstCheck.h"
 #include "NewDeleteOverloadsCheck.h"
 #include "NonCopyableObjects.h"
 #include "NonPrivateMemberVariablesInClassesCheck.h"
 #include "RedundantExpressionCheck.h"
 #include "StaticAssertCheck.h"
 #include "ThrowByValueCatchByReferenceCheck.h"
 #include "UnconventionalAssignOperatorCheck.h"
 #include "UniqueptrResetReleaseCheck.h"
 #include "UnusedAliasDeclsCheck.h"
 #include "UnusedParametersCheck.h"
 #include "UnusedUsingDeclsCheck.h"
 
 namespace clang {
 namespace tidy {
 namespace misc {
 
 class MiscModule : public ClangTidyModule {
 public:
   void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
         "misc-definitions-in-headers");
+    CheckFactories.registerCheck<DispatchOnceAssignmentCheck>(
+        "misc-dispatch-once-assignment");
     CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
     CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
         "misc-new-delete-overloads");
     CheckFactories.registerCheck<NonCopyableObjectsCheck>(
         "misc-non-copyable-objects");
     CheckFactories.registerCheck<NonPrivateMemberVariablesInClassesCheck>(
         "misc-non-private-member-variables-in-classes");
     CheckFactories.registerCheck<RedundantExpressionCheck>(

clang-tools-extra/docs/ReleaseNotes.rst

 --------------------------
 
 - New :doc:`bugprone-dynamic-static-initializers
   <clang-tidy/checks/bugprone-dynamic-static-initializers>` check.
 
   Finds instances where variables with static storage are initialized
   dynamically in header files.
 
+- New :doc:`misc-dispatch-once-assignment
+  <clang-tidy/checks/misc-dispatch-once-assignment>` check.
+
+  Finds instances of explicitly writing to ``dispatch_once_t`` variables.
+
 - New :doc:`linuxkernel-must-use-errs
   <clang-tidy/checks/linuxkernel-must-use-errs>` check.
 
   Checks Linux kernel code to see if it uses the results from the functions in
   ``linux/err.h``.
 
 - New :doc:`google-upgrade-googletest-case
   <clang-tidy/checks/google-upgrade-googletest-case>` check.

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

.. toctree::
    linuxkernel-must-use-errs
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment
    llvm-prefer-isa-or-dyn-cast-in-conditionals
    llvm-prefer-register-over-unsigned
    llvm-twine-local
    misc-definitions-in-headers
+   misc-dispatch-once-assignment
    misc-misplaced-const
    misc-new-delete-overloads
    misc-non-copyable-objects
    misc-non-private-member-variables-in-classes
    misc-redundant-expression
    misc-static-assert
    misc-throw-by-value-catch-by-reference
    misc-unconventional-assign-operator

clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-assignment.rst

+.. title:: clang-tidy - misc-dispatch-once-assignment
+
+misc-dispatch-once-assignment
+=============================
+
+Finds assignments to variables of type ``dispatch_once_t`` and warns to avoid
+assigning to them. Libdispatch intends that such writes should be guarded
+and making such direct writes may potentially violate the run-once protections
+intended by the library.

clang-tools-extra/test/clang-tidy/misc-dispatch-once-assignment.cpp

+// RUN: %check_clang_tidy %s misc-dispatch-once-assignment %t
+
+typedef int dispatch_once_t;
+extern void dispatch_once(dispatch_once_t *pred, void(^block)(void));
+
+static dispatch_once_t onceToken;
+
+void DoOnce(void(^block)(void)) {
+  dispatch_once(&onceToken, block);
+}
+
+void ResetOnce() {
+  onceToken = 0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not assign to dispatch_once_t variables [misc-dispatch-once-assignment]
+}