PaulkaToast updated this revision to Diff 249530. PaulkaToast marked 4 inline comments as done. PaulkaToast added a comment.
Addressed @aaron.ballman comments (: Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75332/new/ https://reviews.llvm.org/D75332 Files: clang-tools-extra/clang-tidy/CMakeLists.txt clang-tools-extra/clang-tidy/ClangTidyForceLinker.h clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/list.rst clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst clang-tools-extra/docs/clang-tidy/index.rst clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers.cpp @@ -0,0 +1,11 @@ +// RUN: %check_clang_tidy %s llvmlibc-restrict-system-libc-headers %t + +#include <stdio.h> +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system libc header stdio.h not allowed +#include <stdlib.h> +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system libc header stdlib.h not allowed +#include "string.h" +// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: system libc header string.h not allowed +#include "stdatomic.h" +#include <stddef.h> +// Compiler provided headers should not throw warnings. Index: clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/llvmlibc-restrict-system-libc-headers-transitive.cpp @@ -0,0 +1,6 @@ +// RUN: %check_clang_tidy %s llvmlibc-restrict-system-libc-headers %t \ +// RUN: -- -header-filter=.* \ +// RUN: -- -I %S/Inputs/llvmlibc + +#include "transitive.h" +// CHECK-MESSAGES: :1:1: warning: system libc header math.h not allowed, transitively included from {{.*}} Index: clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h =================================================================== --- /dev/null +++ clang-tools-extra/test/clang-tidy/checkers/Inputs/llvmlibc/transitive.h @@ -0,0 +1 @@ +#include <math.h> Index: clang-tools-extra/docs/clang-tidy/index.rst =================================================================== --- clang-tools-extra/docs/clang-tidy/index.rst +++ clang-tools-extra/docs/clang-tidy/index.rst @@ -68,6 +68,7 @@ ``google-`` Checks related to Google coding conventions. ``hicpp-`` Checks related to High Integrity C++ Coding Standard. ``llvm-`` Checks related to the LLVM coding conventions. +``llvmlibc-`` Checks related to the LLVM-libc coding standards. ``misc-`` Checks that we didn't have a better category for. ``modernize-`` Checks that advocate usage of modern (currently "modern" means "C++11") language constructs. Index: clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst =================================================================== --- /dev/null +++ clang-tools-extra/docs/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.rst @@ -0,0 +1,20 @@ +.. title:: clang-tidy - llvmlibc-restrict-system-libc-headers + +llvmlibc-restrict-system-libc-headers +===================================== + +Finds includes of system libc headers not provided by the compiler within +llvm-libc implementations. + +.. code-block:: c++ + + #include <stdio.h> // Not allowed because it is part of system libc. + #include <stddef.h> // Allowed because it is provided by the compiler. + #include "internal/stdio.h" // Allowed because it is NOT part of system libc. + + +This check is necessary because accidentally including system libc headers can +lead to subtle and hard to detect bugs. For example consider a system libc +whose ``dirent`` struct has slightly different field ordering than llvm-libc. +While this will compile successfully, this can cause issues during runtime +because they are ABI incompatible. 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 @@ -187,6 +187,7 @@ `llvm-prefer-isa-or-dyn-cast-in-conditionals <llvm-prefer-isa-or-dyn-cast-in-conditionals.html>`_, "Yes" `llvm-prefer-register-over-unsigned <llvm-prefer-register-over-unsigned.html>`_, "Yes" `llvm-twine-local <llvm-twine-local.html>`_, "Yes" + `llvmlibc-restrict-system-libc-headers <llvmlibc-restrict-system-libc-headers.html>`_, `misc-definitions-in-headers <misc-definitions-in-headers.html>`_, "Yes" `misc-misplaced-const <misc-misplaced-const.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_, Index: clang-tools-extra/docs/ReleaseNotes.rst =================================================================== --- clang-tools-extra/docs/ReleaseNotes.rst +++ clang-tools-extra/docs/ReleaseNotes.rst @@ -67,6 +67,12 @@ Improvements to clang-tidy -------------------------- +New module +^^^^^^^^^^ +- New module `llvmlibc`. + + This module contains checks related to the LLVM-libc coding standards. + New checks ^^^^^^^^^^ @@ -88,6 +94,11 @@ Flags use of the `C` standard library functions ``memset``, ``memcpy`` and ``memcmp`` and similar derivatives on non-trivial types. +- New :doc:`llvmlibc-restrict-system-libc-headers + <clang-tidy/checks/llvmlibc-restrict-system-libc-headers>` check. + + Finds includes of sytem libc headers in llvm-libc implementation. + - New :doc:`objc-dealloc-in-category <clang-tidy/checks/objc-dealloc-in-category>` check. Index: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.h @@ -0,0 +1,35 @@ +//===--- RestrictSystemLibcHeadersCheck.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_LLVMLIBC_RESTRICTSYSTEMLIBCHEADERSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_RESTRICTSYSTEMLIBCHEADERSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace llvm_libc { + +/// Warns of accidental inclusions of system libc headers that aren't +/// compiler provided. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/llvmlibc-restrict-system-libc-headers.html +class RestrictSystemLibcHeadersCheck : public ClangTidyCheck { +public: + RestrictSystemLibcHeadersCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP, + Preprocessor *ModuleExpanderPP) override; +}; + +} // namespace llvm_libc +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVMLIBC_RESTRICTSYSTEMLIBCHEADERSCHECK_H Index: clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/llvmlibc/RestrictSystemLibcHeadersCheck.cpp @@ -0,0 +1,73 @@ +//===--- RestrictSystemLibcHeadersCheck.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 "RestrictSystemLibcHeadersCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/HeaderSearch.h" +#include "clang/Lex/HeaderSearchOptions.h" + +namespace clang { +namespace tidy { +namespace llvm_libc { + +namespace { + +class RestrictedIncludesPPCallbacks : public PPCallbacks { +public: + explicit RestrictedIncludesPPCallbacks( + RestrictSystemLibcHeadersCheck &Check, const SourceManager &SM, + const SmallString<128> CompilerIncudeDir) + : Check(Check), SM(SM), CompilerIncudeDir(CompilerIncudeDir) {} + + void InclusionDirective(SourceLocation HashLoc, const Token &IncludeTok, + StringRef FileName, bool IsAngled, + CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, + const Module *Imported, + SrcMgr::CharacteristicKind FileType) override; + +private: + RestrictSystemLibcHeadersCheck &Check; + const SourceManager &SM; + const SmallString<128> CompilerIncudeDir; +}; + +} // namespace + +void RestrictedIncludesPPCallbacks::InclusionDirective( + SourceLocation HashLoc, const Token &IncludeTok, StringRef FileName, + bool IsAngled, CharSourceRange FilenameRange, const FileEntry *File, + StringRef SearchPath, StringRef RelativePath, const Module *Imported, + SrcMgr::CharacteristicKind FileType) { + if (SrcMgr::isSystem(FileType)) { + // Compiler provided headers are allowed (e.g stddef.h). + if (SearchPath == CompilerIncudeDir) return; + if (!SM.isInMainFile(HashLoc)) { + Check.diag( + HashLoc, + "system libc header %0 not allowed, transitively included from %1") + << FileName << SM.getFilename(HashLoc); + } else { + Check.diag(HashLoc, "system libc header %0 not allowed") << FileName; + } + } +} + +void RestrictSystemLibcHeadersCheck::registerPPCallbacks( + const SourceManager &SM, Preprocessor *PP, Preprocessor *ModuleExpanderPP) { + SmallString<128> CompilerIncudeDir = + StringRef(PP->getHeaderSearchInfo().getHeaderSearchOpts().ResourceDir); + llvm::sys::path::append(CompilerIncudeDir, "include"); + PP->addPPCallbacks(std::make_unique<RestrictedIncludesPPCallbacks>( + *this, SM, CompilerIncudeDir)); +} + +} // namespace llvm_libc +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/llvmlibc/LLVMLibcTidyModule.cpp @@ -0,0 +1,37 @@ +//===--- LLVMLibcTidyModule.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 "RestrictSystemLibcHeadersCheck.h" + +namespace clang { +namespace tidy { +namespace llvm_libc { + +class LLVMLibcModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck<RestrictSystemLibcHeadersCheck>( + "llvmlibc-restrict-system-libc-headers"); + } +}; + +// Register the LLVMLibcTidyModule using this statically initialized variable. +static ClangTidyModuleRegistry::Add<LLVMLibcModule> + X("llvmlibc-module", "Adds LLVM libc standards checks."); + +} // namespace llvm_libc + +// This anchor is used to force the linker to link in the generated object file +// and thus register the LLVMLibcModule. +volatile int LLVMLibcModuleAnchorSource = 0; + +} // namespace tidy +} // namespace clang Index: clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt =================================================================== --- /dev/null +++ clang-tools-extra/clang-tidy/llvmlibc/CMakeLists.txt @@ -0,0 +1,15 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyLLVMLibcModule + LLVMLibcTidyModule.cpp + RestrictSystemLibcHeadersCheck.cpp + + LINK_LIBS + clangAST + clangASTMatchers + clangBasic + clangLex + clangTidy + clangTidyUtils + clangTooling + ) Index: clang-tools-extra/clang-tidy/ClangTidyForceLinker.h =================================================================== --- clang-tools-extra/clang-tidy/ClangTidyForceLinker.h +++ clang-tools-extra/clang-tidy/ClangTidyForceLinker.h @@ -45,6 +45,11 @@ static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination = LLVMModuleAnchorSource; +// This anchor is used to force the linker to link the LLVMLibcModule. +extern volatile int LLVMLibcModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED LLVMLibcModuleAnchorDestination = + LLVMLibcModuleAnchorSource; + // This anchor is used to force the linker to link the CppCoreGuidelinesModule. extern volatile int CppCoreGuidelinesModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination = Index: clang-tools-extra/clang-tidy/CMakeLists.txt =================================================================== --- clang-tools-extra/clang-tidy/CMakeLists.txt +++ clang-tools-extra/clang-tidy/CMakeLists.txt @@ -51,6 +51,7 @@ add_subdirectory(hicpp) add_subdirectory(linuxkernel) add_subdirectory(llvm) +add_subdirectory(llvmlibc) add_subdirectory(misc) add_subdirectory(modernize) if(CLANG_ENABLE_STATIC_ANALYZER) @@ -75,6 +76,7 @@ clangTidyHICPPModule clangTidyLinuxKernelModule clangTidyLLVMModule + clangTidyLLVMLibcModule clangTidyMiscModule clangTidyModernizeModule clangTidyObjCModule
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits