[clang-tidy] Add a module for the Linux kernel.

[tmroeder]

Summary

Now that clang is going to be able to build the Linux kernel again on
x86, and we have gen_compile_commands.py upstream for generating
compile_commands.json, clang-tidy can be used on the Linux kernel
source.

To that end, this commit adds a new clang-tidy module to be used for
checks specific to Linux kernel source. The Linux kernel follows its own
style of C, and it will be useful to separate those checks into their
own module.

[tmroeder]

Not sure if you are the right reviewers; I just looked back in the commit history to see who reviewed clang-tidy changes recently.

[lebedev.ri]

Missing docs/ReleaseNotes.rst and docs/clang-tidy/index.rst changes.

clang-tools-extra/clang-tidy/linux/LinuxTidyModule.cpp

13 ↗

(On Diff #192721)

You don't need this here.

[Eugene.Zelenko]

I think linuxkernel name will be less ambiguous.

[alexfh]

Looks reasonable in general, but we usually add modules with at least one check. Let's do the same here.

[tmroeder]

Changed the module name to linuxkernel as suggested and updated the files to match.

[tmroeder]

In D59963#1447985, @alexfh wrote:

Looks reasonable in general, but we usually add modules with at least one check. Let's do the same here.

OK, will do.

clang-tools-extra/clang-tidy/linux/LinuxTidyModule.cpp

13 ↗

(On Diff #192721)

Thanks. I've removed it now.

[alexfh]

Can you verify that the add_new_check.py script works fine with this new module?
Thanks!

[tmroeder]

Updated with an initial check.

Verified that add_new_check.py works (that's how I added the check). Also added some basic docs and a test.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

148 ↗

(On Diff #196148)

Please add short description. Should be same as first statements in documentation.

[tmroeder]

Actually add the documentation in the release notes.

[tmroeder]

Fix a line-length issue in the check code and rewrite the doc text.

[tmroeder]

clang-tools-extra/docs/ReleaseNotes.rst

148 ↗

(On Diff #196148)

Please take a look. I've rewritten the initial documentation lines in the check header and used that text here, as suggested.

[lebedev.ri]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

12 ↗

(On Diff #196151)

This looks wrong

24 ↗

(On Diff #196151)

Are there any rules what kernel considers to be checking and not checking?
This i think e.g. will accept cast-to-void, assignment to a variable and then ignoring it, etc.

[aaron.ballman]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

39 ↗

(On Diff #196151)

Is the presence of the attribute actually important, or is it simply expected that the declarations will have the attribute and so this check is benign?

40 ↗

(On Diff #196151)

clang-tidy diagnostics are nongrammatical in the same way the compiler error messages are: don't capitalize the start of the sentence, no terminating punctuation, etc. I'd probably reword to result from error function %0 is unused

41 ↗

(On Diff #196151)

You should pass the result from getDirectCallee() rather than converting to a string. The diagnostics engine can handle any NamedDecl * automatically, including proper quoting, etc.

48 ↗

(On Diff #196151)

Similar comments here. How about result from function %0 is unused but represents an error value?

49 ↗

(On Diff #196151)

Similar issue here as above.

51 ↗

(On Diff #196151)

Is there a way for the user to explicitly disable this warning, or is there no such thing as a false positive as far as the kernel is concerned?

Should the diagnostic also produce a fix-it, or is there not a sufficiently common replacement pattern to be used?

[gribozavr]

clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst

10 ↗

(On Diff #196151)

IIRC it is possible to pass through compiler warnings through ClangTidy... WDYT about that instead of reimplementing the warning?

However we would lose the ability to "infer" warn_unused_result on functions that return ERR_PTR. However, since the analysis is not cross-translation-unit, IDK how much value there is.

[tmroeder]

Thanks to everyone for the comments. I've answered them as best I can, and I'm definitely open to changes or to scrapping this entirely.

I should have prefixed this patch with a discussion on the main list, perhaps. My main use case for this clang-tidy module is twofold: find problems in existing kernel code and checking incoming patches to (some of the) kernel mailing lists. I don't expect all (or even most) kernel developers to use this, just like I don't think most kernel developers use existing checkers (sparse and smatch) that have to be explicitly enabled by build-time options in Kbuild.

You might ask why I would want to implement these checks in clang-tidy at all if there are already static-analysis tools like sparse and smatch, though I expect that this list is generally in favor of expanding the scope of clang-tidy. The answer is that I think that having these checks directly in the compiler is the right way to go; clang-tidy (and clang-check, where I also would like to add some static analysis for the kernel) provide a principled basis for checking C code rather than using custom C parsers for the kernel. And I really like the AST matcher language and think it provides a powerful tool for writing these checks.

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

12 ↗

(On Diff #196151)

Yeah, I'm not sure where that came from. I'll remove it.

24 ↗

(On Diff #196151)

Yes, this check is extremely simplistic. My understanding of clang-tidy checks was that the most value comes from them catching obviously wrong behavior and not having any false positives. I didn't think they were supposed to catch all the ways in which something could be wrong.

But I've never written a clang-tidy check before. What is their expected purpose?

39 ↗

(On Diff #196151)

The latter. But I think I could remove it; it seems unlikely that the attribute will be removed from these functions any time, though it could be disabled. It gets set in include/linux/compiler-gcc.h because clang sets the macros GNUC and GNUC_MINOR and GNUC_PATCHLEVEL greater than 3, 4, and 0, respectively.

51 ↗

(On Diff #196151)

The basic case (are these error functions being used in any way at all?) is an error and should be fixed. As noted in response to a comment above, the check in that case is so naive that anything it catches is wrong.

With respect to not using the result from functions that return this value, I think the same argument applies: if code calls a function that returns an error like this and literally ignores the result, then clang-tidy should flag it. However, I don't know of a tool that currently checks the kernel for this kind of transitive property with respect to these functions, so it's possible that there are errors like that in the kernel (or idioms that I don't know about).

I thought that the way that you turn off clang-tidy checks is by specifying them at the command line with a minus sign in front of them: -checks=-linuxkernel-must-check-errs.

Or do you mean locally turning it off? In that case, you can just use the result of the function in any trivial way, like casting it to void.

With respect to the fixit; I thought about that, but I'm not sure I know what the right fixit should be. I'd like to leave it without a fixit for now and come back to it later if it becomes clear what to do here.

clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst

10 ↗

(On Diff #196151)

I don't know exactly how to pass the warnings through, but I'd be interested in learning how to do that. I agree that that would be cleaner than this (partial) reimplementation.

Note that my code above does do something like that: I currently check that the unused-result attribute is set on the return from the call.

[tmroeder]

Remove an unnecessary header and fix the error text.

[gribozavr]

clang-tools-extra/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst

10 ↗

(On Diff #196151)

I don't know exactly how to pass the warnings through, but I'd be interested in learning how to do that. I agree that that would be cleaner than this (partial) reimplementation.

It seems like it is done by default, and -Wunused-result is enabled by default:

$ cat /tmp/example.cpp
int __attribute__((warn_unused_result)) foo();

void bar() {
  foo();
}
$ ./bin/clang-tidy /tmp/example.cpp
1 warning generated.
/tmp/example.cpp:4:3: warning: ignoring return value of function declared with 'warn_unused_result' attribute [clang-diagnostic-unused-result]
  foo();
  ^

If it does not work for you, you can enable it on the command line:

clang-tidy -extra-arg=-Wunused-result -checks=clang-diagnostic-unused-result /tmp/example.cpp

Note that my code above does do something like that: I currently check that the unused-result attribute is set on the return from the call.

Yes, I was saying that we would lose the ability to do that. However, is it that valuable? The analysis in ClangTidy does not cross translation unit boundaries, so unless the caller and the callee are in the same file, this inference does not buy us much.

You could also use an existing check, bugprone-unused-return-value, without even requiring the function to be annotated with the attribute:

$ cat /tmp/example.cpp
int foo();

void bar() {
  foo();
}
$ ./bin/clang-tidy /tmp/example.cpp -config="{Checks: 'bugprone-unused-return-value', CheckOptions: [{key: 'bugprone-unused-return-value.CheckedFunctions', value: 'foo'}]}"
1 warning generated.
/tmp/example.cpp:4:3: warning: the value returned by this function should be used [bugprone-unused-return-value]
  foo();
  ^
/tmp/example.cpp:4:3: note: cast the expression to void to silence this warning

[aaron.ballman]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

24 ↗

(On Diff #196151)

We are more forgiving of false positives in clang-tidy than we are in the compiler proper, so we're okay with checks being more chatty, within reason. Obviously, the less false positives (and false negatives), the better because it's more useful for the user.

39 ↗

(On Diff #196151)

I'd probably remove it -- the check is useful even if the function is not marked with the attribute, but if the function is expected to be marked with the attribute in all circumstances anyway, then this is just doing needless work.

51 ↗

(On Diff #196151)

The basic case (are these error functions being used in any way at all?) is an error and should be fixed. As noted in response to a comment above, the check in that case is so naive that anything it catches is wrong.

That sounds great to me.

Or do you mean locally turning it off? In that case, you can just use the result of the function in any trivial way, like casting it to void.

That's mostly what I was trying to understand. We usually want checks to have some way to be silenced locally (cast to void, NOLINT comments, whatever makes sense for the construct), though it sounds like your situation probably doesn't need it because every instance caught is truly a bug.

With respect to the fixit; I thought about that, but I'm not sure I know what the right fixit should be. I'd like to leave it without a fixit for now and come back to it later if it becomes clear what to do here.

Sounds fine to me.

[nickdesaulniers]

clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c

6 ↗

(On Diff #196273)

Let's come up with another check; __must_check has a bug upstream in the kernel sources (I looked into this maybe a month ago). The kernel disables a warning group that this would be under, if I re-enable the lone warning, then this works properly at compile time. (I forget the warning, and should have filed a bug). Point being, fixing this upstream in kernel sources is preferable to me to a clang tidy check, but it's a good start.

[nickdesaulniers]

See also: https://github.com/ClangBuiltLinux/linux/issues/520

[tmroeder]

clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c

6 ↗

(On Diff #196273)

Good point. How about the related smatch checks in https://repo.or.cz/smatch.git/blob_plain/HEAD:/check_err_ptr_deref.c

It looks for cases where possible ERR_PTR values are dereferenced (wrong because it's not a pointer), and passing non-negative values to ERR_PTR.

What do you think?

[dvyukov]

I assume you tried to run it on the kernel. Please post the current output somewhere.

[dvyukov]

Re more checks. I guess we can borrow some from the existing checkers:
https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html
https://bottest.wiki.kernel.org/coccicheck
https://lwn.net/Articles/752408/
https://lwn.net/Articles/691882/

They do some checks very well. But if we could do something better (more true positives, less false positives), that may be useful.
Also figuring out which of the existing checks in clang-tidy/analyzer are relevant/useful for kernel is another direction.
Also making the thread-safety analysis work for kernel may be a big deal.

[nickdesaulniers]

In D59963#1555595, @dvyukov wrote:

Re more checks. I guess we can borrow some from the existing checkers:
https://www.kernel.org/doc/html/v4.12/dev-tools/sparse.html
https://bottest.wiki.kernel.org/coccicheck
https://lwn.net/Articles/752408/
https://lwn.net/Articles/691882/

They do some checks very well. But if we could do something better (more true positives, less false positives), that may be useful.
Also figuring out which of the existing checks in clang-tidy/analyzer are relevant/useful for kernel is another direction.

I agree, but we need the core module added first so we can start adding more.

Also making the thread-safety analysis work for kernel may be a big deal.

Yes; but there's many many issues there; a GSoC project is being done on this.
https://github.com/himanshujha199640/linux-kernel-analysis/blob/report/gsoc19/reports/clang-thread-safety-analysis.md
I don't expect that to be solved here in this code review.

Since we have additional checks waiting on this to land, LGTM.

clang-tools-extra/test/clang-tidy/linuxkernel-must-check-errs.c

6 ↗

(On Diff #196273)

Thinking more about it; while we've now cleaned this up in upstream mainline Linux, it still will be a fair amount of work to backport all of the fixes to the LTS branches from which most distros base their releases on. So this check still will have value in that it can currently detect things that Clang won't report for LTS kernels which are very very relevant to at least Android and CrOS.

Further, @Nathan-Huckleberry has an additional check that needs the module added here, so let's land this patch so we can start adding more checks to the module. Worst case we remove this check, but for now I do think it will give us more coverage of LTS Linux kernels than we have today with Clang.

[tmroeder]

Sync'ing to the latest HEAD commit on master.

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

75 ↗

(On Diff #211783)

Please highlight linux/err.h with double back-ticks. I think will be good idea to synchronize documentation text.

[tmroeder]

Synchronize the documentation, as requested.

[tmroeder]

clang-tools-extra/docs/ReleaseNotes.rst

75 ↗

(On Diff #211783)

Thanks, I've synchronized the documentation now between this file and the header.

[nickdesaulniers]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

12 ↗

(On Diff #196151)

Hard to tell, but I don't think this using statement was ever removed as requested?

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

77 ↗

(On Diff #211836)

Release Notes should include short description. One sentence is enough, but it'll good idea to keep it same as first statement of documentation.

[tmroeder]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

12 ↗

(On Diff #196151)

It's not the "using" statement that was requested to be removed in the initial diff, but rather "#include <c++/8/bits/c++config.h>", which I did remove.

I'm not sure why the "This looks wrong" gets attached to the "using" statement in later diffs.

[tmroeder]

clang-tools-extra/docs/ReleaseNotes.rst

77 ↗

(On Diff #211836)

Would you like me to submit a patch that removes the second paragraph of this description, then?

[Eugene.Zelenko]

clang-tools-extra/docs/ReleaseNotes.rst

77 ↗

(On Diff #211836)

Yes. Documentation is created for details. But you still need to make its first sentence same as in Release Notes. See other checks and their Release Notes as example (in previous release branches).

[tmroeder]

clang-tools-extra/docs/ReleaseNotes.rst

77 ↗

(On Diff #211836)

I'm sorry, I don't understand. What's the referent of "it" in "you still need to make its first sentence same as in Release Notes"? What first sentence needs to match the first sentence of the Release Notes?

If it's the header file documentation (MustCheckErrs.h), then as far as I can tell, the first paragraph of Release Notes is identical to the first paragraph of the .h file documentation, other than the double backquotes, which I didn't think belonged in a .h file comment.

What am I missing?

[tmroeder]

clang-tools-extra/docs/ReleaseNotes.rst

77 ↗

(On Diff #211836)

I have looked back, and I think that https://reviews.llvm.org/D65343 should address the comment here. Please take a look at let me know.

[tmroeder]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

12 ↗

(On Diff #196151)

Nick, can you confirm that this addresses your comment?

[nickdesaulniers]

clang-tools-extra/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

12 ↗

(On Diff #196151)

LGTM

clang-tools-extra/trunk/clang-tidy/CMakeLists.txt

 add_subdirectory(abseil)
 add_subdirectory(boost)
 add_subdirectory(bugprone)
 add_subdirectory(cert)
 add_subdirectory(cppcoreguidelines)
 add_subdirectory(fuchsia)
 add_subdirectory(google)
 add_subdirectory(hicpp)
+add_subdirectory(linuxkernel)
 add_subdirectory(llvm)
 add_subdirectory(misc)
 add_subdirectory(modernize)
 if(CLANG_ENABLE_STATIC_ANALYZER)
   add_subdirectory(mpi)
 endif()
 add_subdirectory(objc)
 add_subdirectory(openmp)
 add_subdirectory(performance)
 add_subdirectory(plugin)
 add_subdirectory(portability)
 add_subdirectory(readability)
 add_subdirectory(tool)
 add_subdirectory(utils)
 add_subdirectory(zircon)

clang-tools-extra/trunk/clang-tidy/ClangTidyForceLinker.h

 static int LLVM_ATTRIBUTE_UNUSED BoostModuleAnchorDestination =
     BoostModuleAnchorSource;
 
 // This anchor is used to force the linker to link the BugproneModule.
 extern volatile int BugproneModuleAnchorSource;
 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 =
     LLVMModuleAnchorSource;
 
 // This anchor is used to force the linker to link the CppCoreGuidelinesModule.
 extern volatile int CppCoreGuidelinesModuleAnchorSource;
 static int LLVM_ATTRIBUTE_UNUSED CppCoreGuidelinesModuleAnchorDestination =

clang-tools-extra/trunk/clang-tidy/linuxkernel/CMakeLists.txt

+set(LLVM_LINK_COMPONENTS support)
+
+add_clang_library(clangTidyLinuxKernelModule
+  LinuxKernelTidyModule.cpp
+  MustCheckErrsCheck.cpp
+
+  LINK_LIBS
+  clangAST
+  clangASTMatchers
+  clangBasic
+  clangLex
+  clangTidy
+  clangTidyUtils
+  )

clang-tools-extra/trunk/clang-tidy/linuxkernel/LinuxKernelTidyModule.cpp

+//===--- 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<MustCheckErrsCheck>(
+        "linuxkernel-must-check-errs");
+  }
+};
+// Register the LinuxKernelTidyModule using this statically initialized
+// variable.
+static ClangTidyModuleRegistry::Add<LinuxKernelModule>
+    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

clang-tools-extra/trunk/clang-tidy/linuxkernel/MustCheckErrsCheck.h

+//===--- 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

clang-tools-extra/trunk/clang-tidy/linuxkernel/MustCheckErrsCheck.cpp

+//===--- 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<CallExpr>("call");
+  if (MatchedCallExpr) {
+    diag(MatchedCallExpr->getExprLoc(), "result from function %0 is unused")
+        << MatchedCallExpr->getDirectCallee();
+  }
+
+  const CallExpr *MatchedTransitiveCallExpr =
+      Result.Nodes.getNodeAs<CallExpr>("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

clang-tools-extra/trunk/clang-tidy/plugin/CMakeLists.txt

add_clang_library(clangTidyPlugin
   clangTidyAndroidModule
   clangTidyBoostModule
   clangTidyBugproneModule
   clangTidyCERTModule
   clangTidyCppCoreGuidelinesModule
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
   clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule

clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt

target_link_libraries(clang-tidy
   clangTidyAbseilModule
   clangTidyBoostModule
   clangTidyBugproneModule
   clangTidyCERTModule
   clangTidyCppCoreGuidelinesModule
   clangTidyFuchsiaModule
   clangTidyGoogleModule
   clangTidyHICPPModule
+  clangTidyLinuxKernelModule
   clangTidyLLVMModule
   clangTidyMiscModule
   clangTidyModernizeModule
   clangTidyObjCModule
   clangTidyOpenMPModule
   clangTidyPerformanceModule
   clangTidyPortabilityModule
   clangTidyReadabilityModule

clang-tools-extra/trunk/docs/ReleaseNotes.rst

 Improvements to clang-rename
 ----------------------------
 
 The improvements are...
 
 Improvements to clang-tidy
 --------------------------
 
-The improvements are...
+- New :doc:`linuxkernel-must-use-errs
+  <clang-tidy/checks/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
 -----------------------------
 
 The improvements are...
 
 Improvements to clang-include-fixer
 -----------------------------------

clang-tools-extra/trunk/docs/clang-tidy/checks/linuxkernel-must-use-errs.rst

+.. 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();

clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

.. toctree::
    hicpp-use-auto (redirects to modernize-use-auto) <hicpp-use-auto>
    hicpp-use-emplace (redirects to modernize-use-emplace) <hicpp-use-emplace>
    hicpp-use-equals-default (redirects to modernize-use-equals-default) <hicpp-use-equals-default>
    hicpp-use-equals-delete (redirects to modernize-use-equals-delete) <hicpp-use-equals-delete>
    hicpp-use-noexcept (redirects to modernize-use-noexcept) <hicpp-use-noexcept>
    hicpp-use-nullptr (redirects to modernize-use-nullptr) <hicpp-use-nullptr>
    hicpp-use-override (redirects to modernize-use-override) <hicpp-use-override>
    hicpp-vararg (redirects to cppcoreguidelines-pro-type-vararg) <hicpp-vararg>
+   linuxkernel-must-use-errs
    llvm-header-guard
    llvm-include-order
    llvm-namespace-comment
    llvm-prefer-isa-or-dyn-cast-in-conditionals
    llvm-twine-local
    misc-definitions-in-headers
    misc-misplaced-const
    misc-new-delete-overloads

clang-tools-extra/trunk/test/clang-tidy/linuxkernel-must-check-errs.c

+// 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
+}