https://github.com/lucasly-ba updated https://github.com/llvm/llvm-project/pull/210548
>From 664c171969080d6f889453df8e5f0eb8e3f101bf Mon Sep 17 00:00:00 2001 From: Lucas Ly Ba <[email protected]> Date: Sat, 18 Jul 2026 23:09:03 +0200 Subject: [PATCH] [clang-tidy] Add `bugprone-errno-comparison` check Comparing 'errno' against an integer literal is not portable because the values of the error constants are implementation-defined. Flag such comparisons and point to the 'E'-prefixed macros instead. Comparisons with 0 (the standard "no error" value) and comparisons whose literal comes from a macro such as errno == EINVAL are not flagged. Fixes #182518 --- .../bugprone/BugproneTidyModule.cpp | 3 + .../clang-tidy/bugprone/CMakeLists.txt | 1 + .../bugprone/ErrnoComparisonCheck.cpp | 56 +++++++++++++++++++ .../bugprone/ErrnoComparisonCheck.h | 37 ++++++++++++ clang-tools-extra/docs/ReleaseNotes.rst | 6 ++ .../checks/bugprone/errno-comparison.rst | 19 +++++++ .../docs/clang-tidy/checks/list.rst | 1 + .../checkers/bugprone/errno-comparison.c | 29 ++++++++++ .../checkers/bugprone/errno-comparison.cpp | 20 +++++++ 9 files changed, 172 insertions(+) create mode 100644 clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.cpp create mode 100644 clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.h create mode 100644 clang-tools-extra/docs/clang-tidy/checks/bugprone/errno-comparison.rst create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.c create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp index 3aa39d10ceb5d..a6b356aeac3a3 100644 --- a/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp @@ -30,6 +30,7 @@ #include "DynamicStaticInitializersCheck.h" #include "EasilySwappableParametersCheck.h" #include "EmptyCatchCheck.h" +#include "ErrnoComparisonCheck.h" #include "ExceptionCopyConstructorThrowsCheck.h" #include "ExceptionEscapeCheck.h" #include "FloatLoopCounterCheck.h" @@ -165,6 +166,8 @@ class BugproneModule : public ClangTidyModule { CheckFactories.registerCheck<EasilySwappableParametersCheck>( "bugprone-easily-swappable-parameters"); CheckFactories.registerCheck<EmptyCatchCheck>("bugprone-empty-catch"); + CheckFactories.registerCheck<ErrnoComparisonCheck>( + "bugprone-errno-comparison"); CheckFactories.registerCheck<ExceptionCopyConstructorThrowsCheck>( "bugprone-exception-copy-constructor-throws"); CheckFactories.registerCheck<ExceptionEscapeCheck>( diff --git a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt index 43e85b1407f21..c6933bc05fcd6 100644 --- a/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt @@ -27,6 +27,7 @@ add_clang_library(clangTidyBugproneModule STATIC DynamicStaticInitializersCheck.cpp EasilySwappableParametersCheck.cpp EmptyCatchCheck.cpp + ErrnoComparisonCheck.cpp ExceptionCopyConstructorThrowsCheck.cpp ExceptionEscapeCheck.cpp FloatLoopCounterCheck.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.cpp new file mode 100644 index 0000000000000..ebec29b26354f --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.cpp @@ -0,0 +1,56 @@ +//===----------------------------------------------------------------------===// +// +// 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 "ErrnoComparisonCheck.h" +#include "clang/AST/Expr.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::bugprone { + +void ErrnoComparisonCheck::registerMatchers(MatchFinder *Finder) { + Finder->addMatcher( + binaryOperator( + isComparisonOperator(), + hasEitherOperand(ignoringParenImpCasts(integerLiteral().bind("lit")))) + .bind("cmp"), + this); +} + +void ErrnoComparisonCheck::check(const MatchFinder::MatchResult &Result) { + const auto *Cmp = Result.Nodes.getNodeAs<BinaryOperator>("cmp"); + const auto *Lit = Result.Nodes.getNodeAs<IntegerLiteral>("lit"); + + // errno == 0 is the portable "no error" test. + if (Lit->getValue() == 0) + return; + + // A literal from a macro is the recommended form, e.g. errno == EINVAL. + if (Lit->getBeginLoc().isMacroID()) + return; + + // True when an operand is spelled with the 'errno' macro. + const SourceManager &SM = *Result.SourceManager; + auto IsErrno = [&](const Expr *E) { + SourceLocation Loc = E->getBeginLoc(); + return Loc.isMacroID() && + Lexer::getImmediateMacroName(Loc, SM, getLangOpts()) == "errno"; + }; + + // Exactly one side must be errno (comparing two literals, or something that + // isn't errno, is none of our business). + if (IsErrno(Cmp->getLHS()) == IsErrno(Cmp->getRHS())) + return; + + diag(Cmp->getOperatorLoc(), + "comparing 'errno' against a literal is not portable"); +} + +} // namespace clang::tidy::bugprone diff --git a/clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.h b/clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.h new file mode 100644 index 0000000000000..f85026ae57c42 --- /dev/null +++ b/clang-tools-extra/clang-tidy/bugprone/ErrnoComparisonCheck.h @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// +// 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_BUGPRONE_ERRNOCOMPARISONCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ERRNOCOMPARISONCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::bugprone { + +/// Flags comparisons of 'errno' against an integer literal. The values of the +/// error constants are implementation-defined, so a literal such as +/// 'errno == 5' is not portable; the 'E'-prefixed macros should be used. +/// +/// For the user-facing documentation see: +/// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/errno-comparison.html +class ErrnoComparisonCheck : public ClangTidyCheck { +public: + ErrnoComparisonCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + // Look at the code as written so a comparison inside a template isn't + // re-reported for every instantiation. + return TK_IgnoreUnlessSpelledInSource; + } +}; + +} // namespace clang::tidy::bugprone + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_ERRNOCOMPARISONCHECK_H diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 69c3bcf67b8db..d900c1b8c1bdd 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -97,6 +97,12 @@ Improvements to clang-tidy New checks ^^^^^^^^^^ +- New :doc:`bugprone-errno-comparison + <clang-tidy/checks/bugprone/errno-comparison>` check. + + Flags comparisons of ``errno`` against an integer literal, which is not + portable because the values of the error constants are implementation-defined. + New check aliases ^^^^^^^^^^^^^^^^^ diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/errno-comparison.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/errno-comparison.rst new file mode 100644 index 0000000000000..7155a31fc7b75 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/errno-comparison.rst @@ -0,0 +1,19 @@ +.. title:: clang-tidy - bugprone-errno-comparison + +bugprone-errno-comparison +========================= + +Flags comparisons of ``errno`` against an integer literal. + +The values of the ``errno`` error constants are implementation-defined, so +comparing ``errno`` against a hard-coded number such as ``errno == 5`` is not +portable. Use the ``E``-prefixed macros (e.g. ``EINVAL``) instead. + +.. code-block:: c + + if (errno == 5) {} // warning + if (errno == EINVAL) {} // ok, compared against the named macro + if (errno == 0) {} // ok, 0 is the standard "no error" value + +Comparisons with ``0`` and comparisons whose literal comes from a macro (such as +the ``E``-prefixed constants themselves) are not flagged. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 2a44dc78fbc89..73270750afb96 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -99,6 +99,7 @@ Clang-Tidy Checks :doc:`bugprone-dynamic-static-initializers <bugprone/dynamic-static-initializers>`, :doc:`bugprone-easily-swappable-parameters <bugprone/easily-swappable-parameters>`, :doc:`bugprone-empty-catch <bugprone/empty-catch>`, + :doc:`bugprone-errno-comparison <bugprone/errno-comparison>`, :doc:`bugprone-exception-copy-constructor-throws <bugprone/exception-copy-constructor-throws>`, :doc:`bugprone-exception-escape <bugprone/exception-escape>`, :doc:`bugprone-float-loop-counter <bugprone/float-loop-counter>`, diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.c new file mode 100644 index 0000000000000..a967464587965 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.c @@ -0,0 +1,29 @@ +// RUN: %check_clang_tidy %s bugprone-errno-comparison %t + +extern int *__errno_location(void); +#define errno (*__errno_location()) +#define EINVAL 22 + +void positive(void) { + if (errno == 5) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [bugprone-errno-comparison] + if (errno != 5) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [bugprone-errno-comparison] + if (errno < 10) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [bugprone-errno-comparison] + if (5 == errno) {} + // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: comparing 'errno' against a literal is not portable [bugprone-errno-comparison] +} + +enum Err { MyErr = 5 }; + +#define CMP_ERR(e) ((e) == 5) + +void negative(int x) { + if (errno == 0) {} // errno == 0 is the portable "no error" check + if (errno != 0) {} + if (errno == EINVAL) {} // comparing against the named macro is the fix + if (errno == MyErr) {} // an enumerator is not an integer literal + if (x == 5) {} // not errno + if (CMP_ERR(errno)) {} // the comparison itself is written in a macro +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.cpp new file mode 100644 index 0000000000000..286cf5e5637c2 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/errno-comparison.cpp @@ -0,0 +1,20 @@ +// RUN: %check_clang_tidy %s bugprone-errno-comparison %t + +extern int *__errno_location(); +#define errno (*__errno_location()) + +// A literal comparison written directly still fires in C++. +void direct() { + if (errno == 5) {} + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: comparing 'errno' against a literal is not portable [bugprone-errno-comparison] +} + +// Comparing against a template parameter is not a literal as written, so no +// warning is issued and the instantiations below don't report it repeatedly. +template <int N> +bool cmp() { + return errno == N; +} + +bool a = cmp<5>(); +bool b = cmp<7>(); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
