[clang-tools-extra] r373065 - [clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage.
Dmitri Gribenko via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 27 03:49:13 PDT 2019
Author: gribozavr
Date: Fri Sep 27 03:49:12 2019
New Revision: 373065
URL: http://llvm.org/viewvc/llvm-project?rev=373065&view=rev
Log:
[clang-tidy] New check to warn when storing dispatch_once_t in non-static, non-global storage.
Summary:
Creates a new darwin ClangTidy module and adds the darwin-dispatch-once-nonstatic check that 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 this issue in clang-tidy when possible.
This is a re-land of https://reviews.llvm.org/D67567
Reviewers: thakis, gribozavr, stephanemoore
Subscribers: Eugene.Zelenko, mgorny, xazax.hun, jkorous, arphaman, kadircet, usaxena95
Tags: #clang-tools-extra, #clang, #llvm
Differential Revision: https://reviews.llvm.org/D68109
Added:
clang-tools-extra/trunk/clang-tidy/darwin/
clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp
clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst
clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Fri Sep 27 03:49:12 2019
@@ -43,6 +43,7 @@ add_subdirectory(boost)
add_subdirectory(bugprone)
add_subdirectory(cert)
add_subdirectory(cppcoreguidelines)
+add_subdirectory(darwin)
add_subdirectory(fuchsia)
add_subdirectory(google)
add_subdirectory(hicpp)
Modified: clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h Fri Sep 27 03:49:12 2019
@@ -50,6 +50,11 @@ extern volatile int CppCoreGuidelinesMod
static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =
CppCoreGuidelinesModuleAnchorSource;
+// This anchor is used to force the linker to link the DarwinModule.
+extern volatile int DarwinModuleAnchorSource;
+static int LLVM_ATTRIBUTE_UNUSED DarwinModuleAnchorDestination =
+ DarwinModuleAnchorSource;
+
// This anchor is used to force the linker to link the FuchsiaModule.
extern volatile int FuchsiaModuleAnchorSource;
static int LLVM_ATTRIBUTE_UNUSED FuchsiaModuleAnchorDestination =
Added: clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt?rev=373065&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt (added)
+++ clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt Fri Sep 27 03:49:12 2019
@@ -0,0 +1,15 @@
+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyDarwinModule
+ DarwinTidyModule.cpp
+ DispatchOnceNonstaticCheck.cpp
+
+ LINK_LIBS
+ clangAnalysis
+ clangAST
+ clangASTMatchers
+ clangBasic
+ clangLex
+ clangTidy
+ clangTidyUtils
+ )
Added: clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp?rev=373065&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp Fri Sep 27 03:49:12 2019
@@ -0,0 +1,37 @@
+//===--- 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 "DispatchOnceNonstaticCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace darwin {
+
+class DarwinModule : public ClangTidyModule {
+public:
+ void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
+ CheckFactories.registerCheck<DispatchOnceNonstaticCheck>(
+ "darwin-dispatch-once-nonstatic");
+ }
+};
+
+} // namespace darwin
+
+// Register the DarwinTidyModule using this statically initialized variable.
+static ClangTidyModuleRegistry::Add<darwin::DarwinModule>
+ X("misc-module", "Adds miscellaneous lint checks.");
+
+// This anchor is used to force the linker to link in the generated object file
+// and thus register the DarwinModule.
+volatile int DarwinModuleAnchorSource = 0;
+
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp?rev=373065&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp Fri Sep 27 03:49:12 2019
@@ -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 darwin {
+
+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_t variables 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 darwin
+} // namespace tidy
+} // namespace clang
Added: clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h?rev=373065&view=auto
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h (added)
+++ clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h Fri Sep 27 03:49:12 2019
@@ -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_DARWIN_DISPATCHONCENONSTATICCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_DARWIN_DISPATCHONCENONSTATICCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang {
+namespace tidy {
+namespace darwin {
+
+/// 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/darwin-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 darwin
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_DARWIN_DISPATCHONCENONSTATICCHECK_H
Modified: clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt Fri Sep 27 03:49:12 2019
@@ -14,6 +14,7 @@ add_clang_library(clangTidyPlugin
clangTidyBugproneModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
+ clangTidyDarwinModule
clangTidyFuchsiaModule
clangTidyGoogleModule
clangTidyHICPPModule
Modified: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt Fri Sep 27 03:49:12 2019
@@ -23,6 +23,7 @@ target_link_libraries(clang-tidy
clangTidyBugproneModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
+ clangTidyDarwinModule
clangTidyFuchsiaModule
clangTidyGoogleModule
clangTidyHICPPModule
Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Fri Sep 27 03:49:12 2019
@@ -123,6 +123,7 @@ add_clang_library(clangDaemon
clangTidyBugproneModule
clangTidyCERTModule
clangTidyCppCoreGuidelinesModule
+ clangTidyDarwinModule
clangTidyFuchsiaModule
clangTidyGoogleModule
clangTidyHICPPModule
Modified: clang-tools-extra/trunk/docs/ReleaseNotes.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/ReleaseNotes.rst?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/ReleaseNotes.rst (original)
+++ clang-tools-extra/trunk/docs/ReleaseNotes.rst Fri Sep 27 03:49:12 2019
@@ -79,6 +79,12 @@ Improvements to clang-tidy
Finds obvious infinite loops (loops where the condition variable is not
changed at all).
+- New :doc:`darwin-dispatch-once-nonstatic
+ <clang-tidy/checks/darwin-dispatch-once-nonstatic>` check.
+
+ Finds declarations of ``dispatch_once_t`` variables without static or global
+ storage.
+
- New :doc:`linuxkernel-must-use-errs
<clang-tidy/checks/linuxkernel-must-use-errs>` check.
Added: clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst?rev=373065&view=auto
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst (added)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst Fri Sep 27 03:49:12 2019
@@ -0,0 +1,22 @@
+.. title:: clang-tidy - darwin-dispatch-once-nonstatic
+
+darwin-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 pattern 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`` variables be
+members of structs or classes, with the intent to lazily perform some expensive
+struct or class member initialization only once; however, this violates the
+libdispatch requirements.
+
+See the discussion section of
+`Apple's dispatch_once documentation <https://developer.apple.com/documentation/dispatch/1447169-dispatch_once>`_
+for more information.
Modified: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst?rev=373065&r1=373064&r2=373065&view=diff
==============================================================================
--- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst (original)
+++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst Fri Sep 27 03:49:12 2019
@@ -212,6 +212,7 @@ Clang-Tidy Checks
cppcoreguidelines-pro-type-vararg
cppcoreguidelines-slicing
cppcoreguidelines-special-member-functions
+ darwin-dispatch-once-nonstatic
fuchsia-default-arguments-calls
fuchsia-default-arguments-declarations
fuchsia-header-anon-namespaces (redirects to google-build-namespaces) <fuchsia-header-anon-namespaces>
Added: clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm?rev=373065&view=auto
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm (added)
+++ clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm Fri Sep 27 03:49:12 2019
@@ -0,0 +1,48 @@
+// RUN: %check_clang_tidy %s darwin-dispatch-once-nonstatic %t -- -- -fblocks
+
+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_t variables must have static or global storage duration; function parameters should be pointer references [darwin-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 [darwin-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)
+};
+
+ at 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 [darwin-dispatch-once-nonstatic]
+ // CHECK-FIXES: dispatch_once_t _once;
+}
+ at end
More information about the cfe-commits
mailing list