mwyman created this revision.
mwyman added reviewers: benhamilton, hokein, stephanemoore.
Herald added subscribers: cfe-commits, mgorny.
Herald added a project: clang.
Adds two new ClangTidy checks:
- misc-dispatch-once-assignment: warns about assignment to a dispatch_once_t
variable. In code reviews I've encountered enough attempts to "reset" a
dispatch_once.
- misc-dispatch-once-nonstatic: warns about dispatch_once_t variables not in
static or global storage. This catches a missing static for local variables in
e.g. singleton initialization behavior, and also warns on storing
dispatch_once_t values in Objective-C instance variables. C/C++ struct/class
instances may potentially live in static/global storage, and are ignored for
this check.
The osx.API static analysis checker can find the non-static storage use of
dispatch_once_t; I thought it useful to also catch these issues in clang-tidy
when possible.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D67567
Files:
clang-tools-extra/clang-tidy/misc/CMakeLists.txt
clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.cpp
clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.h
clang-tools-extra/clang-tidy/misc/DispatchOnceNonstaticCheck.cpp
clang-tools-extra/clang-tidy/misc/DispatchOnceNonstaticCheck.h
clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-assignment.rst
clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-nonstatic.rst
clang-tools-extra/test/clang-tidy/misc-dispatch-once-assignment.cpp
clang-tools-extra/test/clang-tidy/misc-dispatch-once-nonstatic.mm
Index: clang-tools-extra/test/clang-tidy/misc-dispatch-once-nonstatic.mm
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-dispatch-once-nonstatic.mm
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s misc-dispatch-once-nonstatic %t
+
+typedef int dispatch_once_t;
+extern void dispatch_once(dispatch_once_t *pred, void(^block)(void));
+
+
+void bad_dispatch_once(dispatch_once_t once, void(^block)(void)) {}
+// CHECK-MESSAGES: :[[@LINE-1]]:24: warning: dispatch_once_ts must have static or global storage duration; function parameters should be pointer references [misc-dispatch-once-nonstatic]
+
+// file-scope dispatch_once_ts have static storage duration.
+dispatch_once_t global_once;
+static dispatch_once_t file_static_once;
+namespace {
+dispatch_once_t anonymous_once;
+} // end anonymous namespace
+
+int Correct(void) {
+ static int value;
+ static dispatch_once_t once;
+ dispatch_once(&once, ^{
+ value = 1;
+ });
+ return value;
+}
+
+int Incorrect(void) {
+ static int value;
+ dispatch_once_t once;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: dispatch_once_t variables must have static or global storage duration [misc-dispatch-once-nonstatic]
+ // CHECK-FIXES: static dispatch_once_t once;
+ dispatch_once(&once, ^{
+ value = 1;
+ });
+ return value;
+}
+
+struct OnceStruct {
+ static dispatch_once_t staticOnce; // Allowed
+ int value;
+ dispatch_once_t once; // Allowed (at this time)
+};
+
+@interface MyObject {
+ dispatch_once_t _once;
+ // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: dispatch_once_t variables must have static or global storage duration and cannot be Objective-C instance variables [misc-dispatch-once-nonstatic]
+}
+@end
Index: clang-tools-extra/test/clang-tidy/misc-dispatch-once-assignment.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/misc-dispatch-once-assignment.cpp
@@ -0,0 +1,15 @@
+// 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]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-nonstatic.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-nonstatic.rst
@@ -0,0 +1,18 @@
+.. title:: clang-tidy - misc-dispatch-once-nonstatic
+
+misc-dispatch-once-nonstatic
+============================
+
+Finds declarations of ``dispatch_once_t`` variables without static or global
+storage. The behavior of using ``dispatch_once_t`` predicates with automatic
+or dynamic storage is undefined by libdispatch, and should be avoided.
+
+It is a common paradigm to have functions initialize internal static or global
+data once when the function runs, but programmers have been known to miss the
+static on the ``dispatch_once_t`` predicate, leading to an uninitialized flag
+value at the mercy of the stack.
+
+Programmers have also been known to make ``dispatch_once_t``s be members of
+structs/classes, with the intent to lazily perform some expensive struct or
+class member initialization only once; however, this violates the libdispatch
+requirements.
Index: clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-assignment.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/misc-dispatch-once-assignment.rst
@@ -0,0 +1,9 @@
+.. 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.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -278,6 +278,8 @@
llvm-prefer-register-over-unsigned
llvm-twine-local
misc-definitions-in-headers
+ misc-dispatch-once-assignment
+ misc-dispatch-once-nonstatic
misc-misplaced-const
misc-new-delete-overloads
misc-non-copyable-objects
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -73,6 +73,17 @@
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:`misc-dispatch-once-nonstatic
+ <clang-tidy/checks/misc-dispatch-once-nonstatic>` check.
+
+ Finds instances of ``dispatch_once_t`` variables not having static or global
+ storage.
+
- New :doc:`linuxkernel-must-use-errs
<clang-tidy/checks/linuxkernel-must-use-errs>` check.
Index: clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
+++ clang-tools-extra/clang-tidy/misc/MiscTidyModule.cpp
@@ -10,6 +10,8 @@
#include "../ClangTidyModule.h"
#include "../ClangTidyModuleRegistry.h"
#include "DefinitionsInHeadersCheck.h"
+#include "DispatchOnceAssignmentCheck.h"
+#include "DispatchOnceNonstaticCheck.h"
#include "MisplacedConstCheck.h"
#include "NewDeleteOverloadsCheck.h"
#include "NonCopyableObjects.h"
@@ -32,6 +34,10 @@
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
CheckFactories.registerCheck<DefinitionsInHeadersCheck>(
"misc-definitions-in-headers");
+ CheckFactories.registerCheck<DispatchOnceAssignmentCheck>(
+ "misc-dispatch-once-assignment");
+ CheckFactories.registerCheck<DispatchOnceNonstaticCheck>(
+ "misc-dispatch-once-nonstatic");
CheckFactories.registerCheck<MisplacedConstCheck>("misc-misplaced-const");
CheckFactories.registerCheck<NewDeleteOverloadsCheck>(
"misc-new-delete-overloads");
Index: clang-tools-extra/clang-tidy/misc/DispatchOnceNonstaticCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/DispatchOnceNonstaticCheck.h
@@ -0,0 +1,35 @@
+//===--- DispatchOnceNonstaticCheck.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_DISPATCHONCENONSTATICCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MISC_DISPATCHONCENONSTATICCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+/// Finds variables of type dispatch_once_t that do not have static or global
+/// storage duration, as required by the libdispatch documentation.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/misc-dispatch-once-nonstatic.html
+class DispatchOnceNonstaticCheck : public ClangTidyCheck {
+public:
+ DispatchOnceNonstaticCheck(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_DISPATCHONCENONSTATICCHECK_H
Index: clang-tools-extra/clang-tidy/misc/DispatchOnceNonstaticCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/DispatchOnceNonstaticCheck.cpp
@@ -0,0 +1,62 @@
+//===--- DispatchOnceNonstaticCheck.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 "DispatchOnceNonstaticCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclObjC.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/Diagnostic.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace misc {
+
+void DispatchOnceNonstaticCheck::registerMatchers(MatchFinder *Finder) {
+ // Find variables without static or global storage. VarDecls do not include
+ // struct/class members, which are FieldDecls.
+ Finder->addMatcher(
+ varDecl(hasLocalStorage(), hasType(asString("dispatch_once_t")))
+ .bind("non-static-var"),
+ this);
+
+ // Members of structs or classes might be okay, if the use is at static or
+ // global scope. These will be ignored for now. But ObjC ivars can be
+ // flagged immediately, since they cannot be static.
+ Finder->addMatcher(
+ objcIvarDecl(hasType(asString("dispatch_once_t"))).bind("ivar"), this);
+}
+
+void DispatchOnceNonstaticCheck::check(const MatchFinder::MatchResult &Result) {
+ if (const auto *VD = Result.Nodes.getNodeAs<VarDecl>("non-static-var")) {
+ if (const auto *PD = dyn_cast<ParmVarDecl>(VD)) {
+ // Catch function/method parameters, as any dispatch_once_t should be
+ // passed by pointer instead.
+ diag(PD->getTypeSpecStartLoc(),
+ "dispatch_once_ts must have static or global storage duration; "
+ "function parameters should be pointer references");
+ } else {
+ diag(VD->getTypeSpecStartLoc(), "dispatch_once_t variables must have "
+ "static or global storage duration")
+ << FixItHint::CreateInsertion(VD->getTypeSpecStartLoc(), "static ");
+ }
+ }
+
+ if (const auto *D = Result.Nodes.getNodeAs<ObjCIvarDecl>("ivar")) {
+ diag(D->getTypeSpecStartLoc(),
+ "dispatch_once_t variables must have static or global storage "
+ "duration and cannot be Objective-C instance variables");
+ }
+}
+
+} // namespace misc
+} // namespace tidy
+} // namespace clang
Index: clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.h
@@ -0,0 +1,36 @@
+//===--- 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
Index: clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/misc/DispatchOnceAssignmentCheck.cpp
@@ -0,0 +1,40 @@
+//===--- 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
Index: clang-tools-extra/clang-tidy/misc/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/misc/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/misc/CMakeLists.txt
@@ -2,6 +2,8 @@
add_clang_library(clangTidyMiscModule
DefinitionsInHeadersCheck.cpp
+ DispatchOnceAssignmentCheck.cpp
+ DispatchOnceNonstaticCheck.cpp
MiscTidyModule.cpp
MisplacedConstCheck.cpp
NewDeleteOverloadsCheck.cpp
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
