diff --git a/clang-tools-extra/clang-tidy/CMakeLists.txt b/clang-tools-extra/clang-tidy/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/CMakeLists.txt @@ -44,6 +44,7 @@ add_subdirectory(fuchsia) add_subdirectory(google) add_subdirectory(hicpp) +add_subdirectory(linuxkernel) add_subdirectory(llvm) add_subdirectory(misc) add_subdirectory(modernize) diff --git a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h --- a/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h +++ b/clang-tools-extra/clang-tidy/ClangTidyForceLinker.h @@ -35,6 +35,11 @@ static int LLVM_ATTRIBUTE_UNUSED BugproneModuleAnchorDestination = BugproneModuleAnchorSource; +// This anchor is used to force the linker to link the LinuxKernelModule. +extern volatile int LinuxKernelModuleAnchorSource; +static int LLVM_ATTRIBUTE_UNUSED LinuxKernelModuleAnchorDestination = + LinuxKernelModuleAnchorSource; + // This anchor is used to force the linker to link the LLVMModule. extern volatile int LLVMModuleAnchorSource; static int LLVM_ATTRIBUTE_UNUSED LLVMModuleAnchorDestination = diff --git a/clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt b/clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/linuxkernel/CMakeLists.txt @@ -0,0 +1,14 @@ +set(LLVM_LINK_COMPONENTS support) + +add_clang_library(clangTidyLinuxKernelModule + LinuxKernelTidyModule.cpp + MustCheckErrsCheck.cpp + + LINK_LIBS + clangAST + clangASTMatchers + clangBasic + clangLex + clangTidy + clangTidyUtils + ) diff --git a/clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp b/clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp @@ -0,0 +1,37 @@ +//===--- LinuxKernelTidyModule.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 "MustCheckErrsCheck.h" + +namespace clang { +namespace tidy { +namespace linuxkernel { + +/// This module is for checks specific to the Linux kernel. +class LinuxKernelModule : public ClangTidyModule { +public: + void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "linuxkernel-must-check-errs"); + } +}; +// Register the LinuxKernelTidyModule using this statically initialized +// variable. +static ClangTidyModuleRegistry::Add + X("linux-module", "Adds checks specific to the Linux kernel."); +} // namespace linuxkernel + +// This anchor is used to force the linker to link in the generated object file +// and thus register the LinuxKernelModule. +volatile int LinuxKernelModuleAnchorSource = 0; + +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.h b/clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.h new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.h @@ -0,0 +1,43 @@ +//===--- MustCheckErrsCheck.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_LINUXKERNEL_MUSTCHECKERRSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MUSTCHECKERRSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang { +namespace tidy { +namespace linuxkernel { + +/// Checks Linux kernel code to see if it uses the results from the functions in +/// linux/err.h. Also checks to see if code uses the results from functions that +/// directly return a value from one of these error functions. +/// +/// This is important in the Linux kernel because ERR_PTR, PTR_ERR, IS_ERR, +/// IS_ERR_OR_NULL, ERR_CAST, and PTR_ERR_OR_ZERO return values must be checked, +/// since positive pointers and negative error codes are being used in the same +/// context. These functions are marked with +/// __attribute__((warn_unused_result)), but some kernel versions do not have +/// this warning enabled for clang. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/linuxkernel-must-use-errs.html +class MustCheckErrsCheck : public ClangTidyCheck { +public: + MustCheckErrsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace linuxkernel +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LINUXKERNEL_MUSTCHECKERRSCHECK_H diff --git a/clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp b/clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp new file mode 100644 --- /dev/null +++ b/clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp @@ -0,0 +1,53 @@ +//===--- MustCheckErrsCheck.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 "MustCheckErrsCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; + +namespace clang { +namespace tidy { +namespace linuxkernel { + +void MustCheckErrsCheck::registerMatchers(MatchFinder *Finder) { + auto ErrFn = + functionDecl(hasAnyName("ERR_PTR", "PTR_ERR", "IS_ERR", "IS_ERR_OR_NULL", + "ERR_CAST", "PTR_ERR_OR_ZERO")); + auto NonCheckingStmts = stmt(anyOf(compoundStmt(), labelStmt())); + Finder->addMatcher( + callExpr(callee(ErrFn), hasParent(NonCheckingStmts)).bind("call"), + this); + + auto ReturnToCheck = returnStmt(hasReturnValue(callExpr(callee(ErrFn)))); + auto ReturnsErrFn = functionDecl(hasDescendant(ReturnToCheck)); + Finder->addMatcher(callExpr(callee(ReturnsErrFn), hasParent(NonCheckingStmts)) + .bind("transitive_call"), + this); +} + +void MustCheckErrsCheck::check(const MatchFinder::MatchResult &Result) { + const CallExpr *MatchedCallExpr = Result.Nodes.getNodeAs("call"); + if (MatchedCallExpr) { + diag(MatchedCallExpr->getExprLoc(), "result from function %0 is unused") + << MatchedCallExpr->getDirectCallee(); + } + + const CallExpr *MatchedTransitiveCallExpr = + Result.Nodes.getNodeAs("transitive_call"); + if (MatchedTransitiveCallExpr) { + diag(MatchedTransitiveCallExpr->getExprLoc(), + "result from function %0 is unused but represents an error value") + << MatchedTransitiveCallExpr->getDirectCallee(); + } +} + +} // namespace linuxkernel +} // namespace tidy +} // namespace clang diff --git a/clang-tools-extra/clang-tidy/plugin/CMakeLists.txt b/clang-tools-extra/clang-tidy/plugin/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/plugin/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/plugin/CMakeLists.txt @@ -17,6 +17,7 @@ clangTidyFuchsiaModule clangTidyGoogleModule clangTidyHICPPModule + clangTidyLinuxKernelModule clangTidyLLVMModule clangTidyMiscModule clangTidyModernizeModule diff --git a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt --- a/clang-tools-extra/clang-tidy/tool/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/tool/CMakeLists.txt @@ -26,6 +26,7 @@ clangTidyFuchsiaModule clangTidyGoogleModule clangTidyHICPPModule + clangTidyLinuxKernelModule clangTidyLLVMModule clangTidyMiscModule clangTidyModernizeModule diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -67,7 +67,19 @@ Improvements to clang-tidy -------------------------- -The improvements are... +- New :doc:`linuxkernel-must-use-errs + ` check. + + Checks Linux kernel code to see if it uses the results from the functions in + ``linux/err.h``. Also checks to see if code uses the results from functions that + directly return a value from one of these error functions. + + This is important in the Linux kernel because ``ERR_PTR``, ``PTR_ERR``, + ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and ``PTR_ERR_OR_ZERO`` return + values must be checked, since positive pointers and negative error codes are + being used in the same context. These functions are marked with + ``__attribute__((warn_unused_result))``, but some kernel versions do not have + this warning enabled for clang. Improvements to include-fixer ----------------------------- diff --git a/clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst b/clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst @@ -0,0 +1,26 @@ +.. title:: clang-tidy - linuxkernel-must-use-errs + +linuxkernel-must-use-errs +========================= + +Checks for cases where the kernel error functions ``ERR_PTR``, +``PTR_ERR``, ``IS_ERR``, ``IS_ERR_OR_NULL``, ``ERR_CAST``, and +``PTR_ERR_OR_ZERO`` are called but the results are not used. These +functions are marked with ``__attribute__((warn_unused_result))``, but +the compiler warning for this attribute is not always enabled. + +This also checks for unused values returned by functions that return +``ERR_PTR``. + +Examples: + +.. code-block:: c + + /* Trivial unused call to an ERR function */ + PTR_ERR_OR_ZERO(some_function_call()); + + /* A function that returns ERR_PTR. */ + void *fn() { ERR_PTR(-EINVAL); } + + /* An invalid use of fn. */ + fn(); diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -183,6 +183,7 @@ hicpp-use-nullptr (redirects to modernize-use-nullptr) hicpp-use-override (redirects to modernize-use-override) hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) + linuxkernel-must-use-errs llvm-header-guard llvm-include-order llvm-namespace-comment diff --git a/clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c b/clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c @@ -0,0 +1,43 @@ +// RUN: %check_clang_tidy %s linuxkernel-must-check-errs %t + +#define __must_check __attribute__((warn_unused_result)) + +// Prototypes of the error functions. +void * __must_check ERR_PTR(long error); +long __must_check PTR_ERR(const void *ptr); +int __must_check IS_ERR(const void *ptr); +int __must_check IS_ERR_OR_NULL(const void *ptr); +void * __must_check ERR_CAST(const void *ptr); +int __must_check PTR_ERR_OR_ZERO(const void *ptr); + +void f() { + ERR_PTR(0); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'ERR_PTR' is unused + PTR_ERR((void *)0); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'PTR_ERR' is unused + IS_ERR((void *)0); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'IS_ERR' is unused + ERR_CAST((void *)0); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'ERR_CAST' is unused +out: + PTR_ERR_OR_ZERO((void *)0); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'PTR_ERR_OR_ZERO' is unused +} + +void *f1() { + return ERR_PTR(0); +} + +long f2() { + if (IS_ERR((void *)0)) { + return PTR_ERR((void *)0); + } + return -1; +} + +void f3() { + f1(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'f1' is unused but represents an error value + f2(); + // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: result from function 'f2' is unused but represents an error value +}