diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp new file mode 100644 index 0000000000000..f10b97820f4c7 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.cpp @@ -0,0 +1,82 @@ +//===--- AvoidBoundsErrorsCheck.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 "AvoidBoundsErrorsCheck.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" +#include "clang/Lex/Lexer.h" + +using namespace clang::ast_matchers; + +namespace clang::tidy::cppcoreguidelines { + +const CXXMethodDecl *findAlternative(const CXXRecordDecl *MatchedParent, + const CXXMethodDecl *MatchedOperator) { + for (const CXXMethodDecl *Method : MatchedParent->methods()) { + const bool CorrectName = Method->getNameInfo().getAsString() == "at"; + if (!CorrectName) + continue; + + const bool SameReturnType = + Method->getReturnType() == MatchedOperator->getReturnType(); + if (!SameReturnType) + continue; + + const bool SameNumberOfArguments = + Method->getNumParams() == MatchedOperator->getNumParams(); + if (!SameNumberOfArguments) + continue; + + for (unsigned a = 0; a < Method->getNumParams(); a++) { + const bool SameArgType = + Method->parameters()[a]->getOriginalType() == + MatchedOperator->parameters()[a]->getOriginalType(); + if (!SameArgType) + continue; + } + + return Method; + } + return static_cast(nullptr); +} + +void AvoidBoundsErrorsCheck::registerMatchers(MatchFinder *Finder) { + // Need a callExpr here to match CXXOperatorCallExpr ``(&a)->operator[](0)`` + // and CXXMemberCallExpr ``a[0]``. + Finder->addMatcher( + callExpr( + callee( + cxxMethodDecl(hasOverloadedOperatorName("[]")).bind("operator")), + callee(cxxMethodDecl(hasParent( + cxxRecordDecl(hasMethod(hasName("at"))).bind("parent"))))) + .bind("caller"), + this); +} + +void AvoidBoundsErrorsCheck::check(const MatchFinder::MatchResult &Result) { + const ASTContext &Context = *Result.Context; + const SourceManager &Source = Context.getSourceManager(); + const CallExpr *MatchedExpr = Result.Nodes.getNodeAs("caller"); + const CXXMethodDecl *MatchedOperator = + Result.Nodes.getNodeAs("operator"); + const CXXRecordDecl *MatchedParent = + Result.Nodes.getNodeAs("parent"); + + const CXXMethodDecl *Alternative = + findAlternative(MatchedParent, MatchedOperator); + if (!Alternative) + return; + + const SourceLocation AlternativeSource(Alternative->getBeginLoc()); + + diag(MatchedExpr->getBeginLoc(), + "found possibly unsafe operator[], consider using at() instead"); + diag(Alternative->getBeginLoc(), "alternative at() defined here", + DiagnosticIDs::Note); +} + +} // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h new file mode 100644 index 0000000000000..12c7852877123 --- /dev/null +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidBoundsErrorsCheck.h @@ -0,0 +1,35 @@ +//===--- AvoidBoundsErrorsCheck.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_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H + +#include "../ClangTidyCheck.h" + +namespace clang::tidy::cppcoreguidelines { + +/// Enforce CPP core guidelines SL.con.3 +/// +/// See +/// https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#slcon3-avoid-bounds-errors +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.html +class AvoidBoundsErrorsCheck : public ClangTidyCheck { +public: + AvoidBoundsErrorsCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { + return LangOpts.CPlusPlus; + } + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace clang::tidy::cppcoreguidelines + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_CPPCOREGUIDELINES_AVOIDBOUNDSERRORSCHECK_H diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt index eb35bbc6a538f..4972d20417928 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt @@ -4,6 +4,7 @@ set(LLVM_LINK_COMPONENTS ) add_clang_library(clangTidyCppCoreGuidelinesModule + AvoidBoundsErrorsCheck.cpp AvoidCapturingLambdaCoroutinesCheck.cpp AvoidConstOrRefDataMembersCheck.cpp AvoidDoWhileCheck.cpp diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp index e9f0201615616..525bbc7a42ada 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp @@ -19,6 +19,7 @@ #include "../performance/NoexceptMoveConstructorCheck.h" #include "../performance/NoexceptSwapCheck.h" #include "../readability/MagicNumbersCheck.h" +#include "AvoidBoundsErrorsCheck.h" #include "AvoidCapturingLambdaCoroutinesCheck.h" #include "AvoidConstOrRefDataMembersCheck.h" #include "AvoidDoWhileCheck.h" @@ -57,6 +58,8 @@ namespace cppcoreguidelines { class CppCoreGuidelinesModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "cppcoreguidelines-avoid-bounds-errors"); CheckFactories.registerCheck( "cppcoreguidelines-avoid-capturing-lambda-coroutines"); CheckFactories.registerCheck( diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 5b1feffb89ea0..d2bb59b5f89c4 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -131,6 +131,11 @@ New checks to reading out-of-bounds data due to inadequate or incorrect string null termination. +- New :doc:`cppcoreguidelines-avoid-bounds-errors + ` check. + + Flags the unsafe ``operator[]`` and replaces it with ``at()``. + - New :doc:`modernize-use-designated-initializers ` check. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst new file mode 100644 index 0000000000000..13683ee9b5a46 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-bounds-errors.rst @@ -0,0 +1,21 @@ +.. title:: clang-tidy - cppcoreguidelines-avoid-bounds-errors + +cppcoreguidelines-avoid-bounds-errors +===================================== + +This check flags all uses of ``operator[]`` on ``std::vector``, ``std::array``, ``std::deque``, ``std::map``, ``std::unordered_map``, and ``std::flat_map`` and suggests to replace it with ``at()``. +Note that ``std::span`` and ``std::mdspan`` do not support ``at()`` as of C++23, so the use of ``operator[]`` is not flagged. + +For example the code + +.. code-block:: c++ + std::array a; + int b = a[4]; + +will be replaced by + +.. code-block:: c++ + std::vector a; + int b = a.at(4); + +This check enforces the `SL.con.3 ` guideline. diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst index 5d9d487f75f9c..8b472fca2d8ca 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -174,6 +174,7 @@ Clang-Tidy Checks :doc:`cert-oop58-cpp `, :doc:`concurrency-mt-unsafe `, :doc:`concurrency-thread-canceltype-asynchronous `, + :doc:`cppcoreguidelines-avoid-bounds-errors `, "Yes" :doc:`cppcoreguidelines-avoid-capturing-lambda-coroutines `, :doc:`cppcoreguidelines-avoid-const-or-ref-data-members `, :doc:`cppcoreguidelines-avoid-do-while `, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp new file mode 100644 index 0000000000000..5e12e7d71790d --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-bounds-errors.cpp @@ -0,0 +1,65 @@ +namespace std { + template + struct array { + T operator[](unsigned i) { + return T{1}; + } + T at(unsigned i) { + return T{1}; + } + }; + + template + struct unique_ptr { + T operator[](unsigned i) { + return T{1}; + } + }; + + template + struct span { + T operator[](unsigned i) { + return T{1}; + } + }; +} // namespace std + +namespace json { + template + struct node{ + T operator[](unsigned i) { + return T{1}; + } + }; +} // namespace json + + +// RUN: %check_clang_tidy %s cppcoreguidelines-avoid-bounds-errors %t +std::array a; + +auto b = a[0]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +auto c = a[1+1]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +constexpr int index = 1; +auto d = a[index]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] + +int e(int index) { + return a[index]; +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] +} + +auto f = (&a)->operator[](1); +// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: found possibly unsafe operator[], consider using at() instead [cppcoreguidelines-avoid-bounds-errors] + +auto g = a.at(0); + +std::unique_ptr p; +auto q = p[0]; + +std::span s; +auto t = s[0]; + +json::node n; +auto m = n[0];