Index: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt @@ -43,6 +43,7 @@ add_subdirectory(bugprone) add_subdirectory(cert) add_subdirectory(cppcoreguidelines) +add_subdirectory(darwin) add_subdirectory(fuchsia) add_subdirectory(google) add_subdirectory(hicpp) Index: clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h +++ clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h @@ -50,6 +50,11 @@ 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 = Index: clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/darwin/CMakeLists.txt @@ -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 + ) Index: clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp +++ clang-tools-extra/trunk/clang-tidy/darwin/DarwinTidyModule.cpp @@ -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( + "darwin-dispatch-once-nonstatic"); + } +}; + +} // namespace darwin + +// Register the DarwinTidyModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add + 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 Index: clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h =================================================================== --- clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.h +++ clang-tools-extra/trunk/clang-tidy/darwin/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_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 Index: clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/darwin/DispatchOnceNonstaticCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/darwin/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 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("non-static-var")) { + if (const auto *PD = dyn_cast(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("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 Index: clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt @@ -14,6 +14,7 @@ clangTidyBugproneModule clangTidyCERTModule clangTidyCppCoreGuidelinesModule + clangTidyDarwinModule clangTidyFuchsiaModule clangTidyGoogleModule clangTidyHICPPModule Index: clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt +++ clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt @@ -23,6 +23,7 @@ clangTidyBugproneModule clangTidyCERTModule clangTidyCppCoreGuidelinesModule + clangTidyDarwinModule clangTidyFuchsiaModule clangTidyGoogleModule clangTidyHICPPModule Index: clang-tools-extra/trunk/clangd/CMakeLists.txt =================================================================== --- clang-tools-extra/trunk/clangd/CMakeLists.txt +++ clang-tools-extra/trunk/clangd/CMakeLists.txt @@ -123,6 +123,7 @@ clangTidyBugproneModule clangTidyCERTModule clangTidyCppCoreGuidelinesModule + clangTidyDarwinModule clangTidyFuchsiaModule clangTidyGoogleModule clangTidyHICPPModule Index: clang-tools-extra/trunk/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/trunk/docs/ReleaseNotes.rst +++ clang-tools-extra/trunk/docs/ReleaseNotes.rst @@ -79,6 +79,12 @@ Finds obvious infinite loops (loops where the condition variable is not changed at all). +- New :doc:`darwin-dispatch-once-nonstatic + ` check. + + Finds declarations of ``dispatch_once_t`` variables without static or global + storage. + - New :doc:`linuxkernel-must-use-errs ` check. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst @@ -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 `_ +for more information. Index: clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst =================================================================== --- clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst +++ clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst @@ -212,6 +212,7 @@ 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) Index: clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm +++ clang-tools-extra/trunk/test/clang-tidy/darwin-dispatch-once-nonstatic.mm @@ -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) +}; + +@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; +} +@end