https://github.com/Discookie updated https://github.com/llvm/llvm-project/pull/106350
>From c4e05bdb36e270cbf0557f38fad7c04edf011905 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 28 Aug 2024 08:47:20 +0000 Subject: [PATCH 1/3] [clang-tidy] Add user-defined functions to bugprone-unsafe-functions check --- .../bugprone/UnsafeFunctionsCheck.cpp | 225 +++++++++++++++--- .../bugprone/UnsafeFunctionsCheck.h | 12 + .../checks/bugprone/unsafe-functions.rst | 49 ++++ .../bugprone/unsafe-functions-custom.c | 38 +++ .../checkers/bugprone/unsafe-functions.c | 6 + 5 files changed, 295 insertions(+), 35 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index ea7eaa0b0ff811..05c2063402b080 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -7,6 +7,8 @@ //===----------------------------------------------------------------------===// #include "UnsafeFunctionsCheck.h" +#include "../utils/Matchers.h" +#include "../utils/OptionsUtils.h" #include "clang/AST/ASTContext.h" #include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Lex/PPCallbacks.h" @@ -18,6 +20,12 @@ using namespace llvm; namespace clang::tidy::bugprone { +static constexpr llvm::StringLiteral OptionNameCustomNormalFunctions = + "CustomNormalFunctions"; +static constexpr llvm::StringLiteral OptionNameCustomAnnexKFunctions = + "CustomAnnexKFunctions"; +static constexpr llvm::StringLiteral OptionNameReportDefaultFunctions = + "ReportDefaultFunctions"; static constexpr llvm::StringLiteral OptionNameReportMoreUnsafeFunctions = "ReportMoreUnsafeFunctions"; @@ -26,6 +34,10 @@ static constexpr llvm::StringLiteral FunctionNamesWithAnnexKReplacementId = static constexpr llvm::StringLiteral FunctionNamesId = "FunctionsNames"; static constexpr llvm::StringLiteral AdditionalFunctionNamesId = "AdditionalFunctionsNames"; +static constexpr llvm::StringLiteral CustomFunctionNamesId = + "CustomFunctionNames"; +static constexpr llvm::StringLiteral CustomAnnexKFunctionNamesId = + "CustomAnnexKFunctionNames"; static constexpr llvm::StringLiteral DeclRefId = "DRE"; static std::optional<std::string> @@ -127,57 +139,155 @@ static bool isAnnexKAvailable(std::optional<bool> &CacheVar, Preprocessor *PP, return CacheVar.value(); } +static std::vector<UnsafeFunctionsCheck::CheckedFunction> +ParseCheckedFunctions(StringRef Option, StringRef OptionName, + ClangTidyContext *Context) { + std::vector<StringRef> Functions = utils::options::parseStringList(Option); + std::vector<UnsafeFunctionsCheck::CheckedFunction> Result; + Result.reserve(Functions.size()); + + for (StringRef Function : Functions) { + if (Function.empty()) { + continue; + } + + auto [Name, Rest] = Function.split(','); + auto [Replacement, Reason] = Rest.split(','); + + if (Name.trim().empty()) { + Context->configurationDiag("invalid configuration value for option '%0'; " + "expected the name of an unsafe function") + << OptionName; + } + + if (Replacement.trim().empty()) { + Context->configurationDiag( + "invalid configuration value '%0' for option '%1'; " + "expected a replacement function name") + << Name.trim() << OptionName; + } + + Result.push_back({Name.trim().str(), llvm::Regex(Name.trim()), + Replacement.trim().str(), Reason.trim().str()}); + } + + return Result; +} + +static std::string SerializeCheckedFunctions( + const std::vector<UnsafeFunctionsCheck::CheckedFunction> &Functions) { + std::vector<std::string> Result; + Result.reserve(Functions.size()); + + for (const auto &Entry : Functions) { + if (Entry.Reason.empty()) + Result.push_back(Entry.Name + "," + Entry.Replacement); + else + Result.push_back(Entry.Name + "," + Entry.Replacement + "," + + Entry.Reason); + } + + return llvm::join(Result, ";"); +} + UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name, ClangTidyContext *Context) : ClangTidyCheck(Name, Context), + CustomNormalFunctions(ParseCheckedFunctions( + Options.get(OptionNameCustomNormalFunctions, ""), + OptionNameCustomNormalFunctions, Context)), + CustomAnnexKFunctions(ParseCheckedFunctions( + Options.get(OptionNameCustomAnnexKFunctions, ""), + OptionNameCustomAnnexKFunctions, Context)), + ReportDefaultFunctions( + Options.get(OptionNameReportDefaultFunctions, true)), ReportMoreUnsafeFunctions( Options.get(OptionNameReportMoreUnsafeFunctions, true)) {} void UnsafeFunctionsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, OptionNameCustomNormalFunctions, + SerializeCheckedFunctions(CustomNormalFunctions)); + Options.store(Opts, OptionNameCustomAnnexKFunctions, + SerializeCheckedFunctions(CustomAnnexKFunctions)); + Options.store(Opts, OptionNameReportDefaultFunctions, ReportDefaultFunctions); Options.store(Opts, OptionNameReportMoreUnsafeFunctions, ReportMoreUnsafeFunctions); } void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) { - if (getLangOpts().C11) { - // Matching functions with safe replacements only in Annex K. - auto FunctionNamesWithAnnexKReplacementMatcher = hasAnyName( - "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen", "::fscanf", - "::fwprintf", "::fwscanf", "::getenv", "::gmtime", "::localtime", - "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove", "::memset", - "::printf", "::qsort", "::scanf", "::snprintf", "::sprintf", "::sscanf", - "::strcat", "::strcpy", "::strerror", "::strlen", "::strncat", - "::strncpy", "::strtok", "::swprintf", "::swscanf", "::vfprintf", - "::vfscanf", "::vfwprintf", "::vfwscanf", "::vprintf", "::vscanf", - "::vsnprintf", "::vsprintf", "::vsscanf", "::vswprintf", "::vswscanf", - "::vwprintf", "::vwscanf", "::wcrtomb", "::wcscat", "::wcscpy", - "::wcslen", "::wcsncat", "::wcsncpy", "::wcsrtombs", "::wcstok", - "::wcstombs", "::wctomb", "::wmemcpy", "::wmemmove", "::wprintf", - "::wscanf"); + if (ReportDefaultFunctions) { + if (getLangOpts().C11) { + // Matching functions with safe replacements only in Annex K. + auto FunctionNamesWithAnnexKReplacementMatcher = hasAnyName( + "::bsearch", "::ctime", "::fopen", "::fprintf", "::freopen", + "::fscanf", "::fwprintf", "::fwscanf", "::getenv", "::gmtime", + "::localtime", "::mbsrtowcs", "::mbstowcs", "::memcpy", "::memmove", + "::memset", "::printf", "::qsort", "::scanf", "::snprintf", + "::sprintf", "::sscanf", "::strcat", "::strcpy", "::strerror", + "::strlen", "::strncat", "::strncpy", "::strtok", "::swprintf", + "::swscanf", "::vfprintf", "::vfscanf", "::vfwprintf", "::vfwscanf", + "::vprintf", "::vscanf", "::vsnprintf", "::vsprintf", "::vsscanf", + "::vswprintf", "::vswscanf", "::vwprintf", "::vwscanf", "::wcrtomb", + "::wcscat", "::wcscpy", "::wcslen", "::wcsncat", "::wcsncpy", + "::wcsrtombs", "::wcstok", "::wcstombs", "::wctomb", "::wmemcpy", + "::wmemmove", "::wprintf", "::wscanf"); + Finder->addMatcher( + declRefExpr(to(functionDecl(FunctionNamesWithAnnexKReplacementMatcher) + .bind(FunctionNamesWithAnnexKReplacementId))) + .bind(DeclRefId), + this); + } + + // Matching functions with replacements without Annex K. + auto FunctionNamesMatcher = + hasAnyName("::asctime", "asctime_r", "::gets", "::rewind", "::setbuf"); Finder->addMatcher( - declRefExpr(to(functionDecl(FunctionNamesWithAnnexKReplacementMatcher) - .bind(FunctionNamesWithAnnexKReplacementId))) + declRefExpr( + to(functionDecl(FunctionNamesMatcher).bind(FunctionNamesId))) .bind(DeclRefId), this); + + if (ReportMoreUnsafeFunctions) { + // Matching functions with replacements without Annex K, at user request. + auto AdditionalFunctionNamesMatcher = + hasAnyName("::bcmp", "::bcopy", "::bzero", "::getpw", "::vfork"); + Finder->addMatcher( + declRefExpr(to(functionDecl(AdditionalFunctionNamesMatcher) + .bind(AdditionalFunctionNamesId))) + .bind(DeclRefId), + this); + } } - // Matching functions with replacements without Annex K. - auto FunctionNamesMatcher = - hasAnyName("::asctime", "asctime_r", "::gets", "::rewind", "::setbuf"); - Finder->addMatcher( - declRefExpr(to(functionDecl(FunctionNamesMatcher).bind(FunctionNamesId))) - .bind(DeclRefId), - this); - - if (ReportMoreUnsafeFunctions) { - // Matching functions with replacements without Annex K, at user request. - auto AdditionalFunctionNamesMatcher = - hasAnyName("::bcmp", "::bcopy", "::bzero", "::getpw", "::vfork"); - Finder->addMatcher( - declRefExpr(to(functionDecl(AdditionalFunctionNamesMatcher) - .bind(AdditionalFunctionNamesId))) - .bind(DeclRefId), - this); + if (!CustomAnnexKFunctions.empty()) { + std::vector<llvm::StringRef> FunctionNames; + FunctionNames.reserve(CustomAnnexKFunctions.size()); + + for (const auto &Entry : CustomAnnexKFunctions) + FunctionNames.push_back(Entry.Name); + + auto CustomAnnexKFunctionsMatcher = + matchers::matchesAnyListedName(FunctionNames); + + Finder->addMatcher(declRefExpr(to(functionDecl(CustomAnnexKFunctionsMatcher) + .bind(CustomAnnexKFunctionNamesId))) + .bind(DeclRefId), + this); + } + + if (!CustomNormalFunctions.empty()) { + std::vector<llvm::StringRef> FunctionNames; + FunctionNames.reserve(CustomNormalFunctions.size()); + + for (const auto &Entry : CustomNormalFunctions) + FunctionNames.push_back(Entry.Name); + + auto CustomFunctionsMatcher = matchers::matchesAnyListedName(FunctionNames); + + Finder->addMatcher(declRefExpr(to(functionDecl(CustomFunctionsMatcher) + .bind(CustomFunctionNamesId))) + .bind(DeclRefId), + this); } } @@ -186,16 +296,61 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl()); assert(DeclRef && FuncDecl && "No valid matched node in check()"); + // Only one of these are matched at a time. const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>( FunctionNamesWithAnnexKReplacementId); const auto *Normal = Result.Nodes.getNodeAs<FunctionDecl>(FunctionNamesId); const auto *Additional = Result.Nodes.getNodeAs<FunctionDecl>(AdditionalFunctionNamesId); - assert((AnnexK || Normal || Additional) && "No valid match category."); + const auto *CustomAnnexK = + Result.Nodes.getNodeAs<FunctionDecl>(CustomAnnexKFunctionNamesId); + const auto *CustomNormal = + Result.Nodes.getNodeAs<FunctionDecl>(CustomFunctionNamesId); + assert((AnnexK || Normal || Additional || CustomAnnexK || CustomNormal) && + "No valid match category."); bool AnnexKIsAvailable = isAnnexKAvailable(IsAnnexKAvailable, PP, getLangOpts()); StringRef FunctionName = FuncDecl->getName(); + + if (CustomAnnexK || CustomNormal) { + const auto ShowCheckedFunctionWarning = [&](const CheckedFunction &Entry) { + StringRef Reason = + Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str(); + diag(DeclRef->getExprLoc(), "function %0 %1; '%2' should be used instead") + << FuncDecl << Reason << Entry.Replacement + << DeclRef->getSourceRange(); + }; + + if (AnnexKIsAvailable) { + for (const auto &Entry : CustomAnnexKFunctions) { + if (Entry.Pattern.match(FunctionName)) { + // If both Annex K and Normal are matched, show Annex K warning only. + if (CustomAnnexK) + ShowCheckedFunctionWarning(Entry); + + return; + } + } + + assert(!CustomAnnexK && "No custom Annex K function was matched."); + } + + // Annex K was not available, or the assertion failed. + if (CustomAnnexK) + return; + + for (const auto &Entry : CustomNormalFunctions) { + if (Entry.Pattern.match(FunctionName)) { + ShowCheckedFunctionWarning(Entry); + return; + } + } + + assert(false && "No custom function was matched."); + return; + } + const std::optional<std::string> ReplacementFunctionName = [&]() -> std::optional<std::string> { if (AnnexK) { diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h index 5adfee60d1a7de..eb2656096256ba 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h @@ -32,7 +32,19 @@ class UnsafeFunctionsCheck : public ClangTidyCheck { Preprocessor *ModuleExpanderPP) override; void onEndOfTranslationUnit() override; + struct CheckedFunction { + std::string Name; + llvm::Regex Pattern; + std::string Replacement; + std::string Reason; + }; + private: + const std::vector<CheckedFunction> CustomNormalFunctions; + const std::vector<CheckedFunction> CustomAnnexKFunctions; + + // If true, the default set of functions are reported. + const bool ReportDefaultFunctions; /// If true, additional functions from widely used API-s (such as POSIX) are /// added to the list of reported functions. const bool ReportMoreUnsafeFunctions; diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst index a0a267883b6fe9..4402eba2647978 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst @@ -19,6 +19,9 @@ The check implements the following rules from the CERT C Coding Standard: Unsafe functions ---------------- +The following functions are reported if `ReportDefaultFunctions +<unsafe-functions.html#cmdoption-arg-ReportDefaultFunctions>`_ is enabled. + If *Annex K.* is available, a replacement from *Annex K.* is suggested for the following functions: @@ -74,6 +77,36 @@ Both macros have to be defined to suggest replacement functions from *Annex K.* ``__STDC_WANT_LIB_EXT1__`` must be defined to ``1`` by the user **before** including any system headers. +Custom functions +---------------- + +The options `CustomNormalFunctions` and `CustomAnnexKFunctions` allow the user +to define custom functions to be checked. The format is the following, without +newlines: + +.. code:: + + bugprone-unsafe-functions.CustomNormalFunctions=" + functionRegex1, replacement1[, reason1]; + functionRegex2, replacement2[, reason2]; + ... + " + +The functions are matched using POSIX extended regular expressions. +*(Note: The regular expressions do not support negative* ``(?!)`` *matches)* + +The `reason` is optional and is used to provide additional information about the +reasoning behind the replacement. The default reason is ``is marked as unsafe``. + +As an example, the configuration ``^original$, replacement, is deprecated;`` +will produce the following diagnostic message. + +.. code:: c + + original(); // warning: function 'original' is deprecated; 'replacement' should be used instead. + ::std::original(); // no-warning + original_function(); // no-warning + Options ------- @@ -86,6 +119,22 @@ Options this option enables. Default is `true`. +.. option:: ReportDefaultFunctions + + When `true`, the check reports the default set of functions. + Default is `true`. + +.. option:: CustomNormalFunctions + + A comma-separated list of regular expressions, their replacements, and an + optional reason. For more information, see `Custom functions + <unsafe-functions.html#custom-functions>`_. + +.. option:: CustomAnnexKFunctions + + A comma-separated list of regular expressions, their replacements, and an + optional reason. For more information, see `Custom functions + <unsafe-functions.html#custom-functions>`_. Examples -------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c new file mode 100644 index 00000000000000..ab70970bf749c3 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c @@ -0,0 +1,38 @@ +// This test fails on "x86_64-sie" buildbot and "x86_64-scei-ps4" target. +// According to @dyung, something related to the kind of standard library +// availability is causing the failure. Even though we explicitly define +// the relevant macros the check is hunting for in the invocation, the real +// parsing and preprocessor state will not have that case. +// UNSUPPORTED: target={{.*-(ps4|ps5)}} +// +// RUN: %check_clang_tidy -check-suffix=WITHOUT-ANNEX-K %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomNormalFunctions: '^non_annex_k_only$,replacement;^both$,replacement,is a normal function', bugprone-unsafe-functions.CustomAnnexKFunctions: '^annex_k_only$,replacement;^both$,replacement,is an Annex K function'}}"\ +// RUN: -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomNormalFunctions: '^non_annex_k_only$,replacement;^both$,replacement,is a normal function', bugprone-unsafe-functions.CustomAnnexKFunctions: '^annex_k_only$,replacement;^both$,replacement,is an Annex K function'}}"\ +// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 +// RUN: %check_clang_tidy -check-suffix=WITH-ONLY-ANNEX-K %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomAnnexKFunctions: '^annex_k_only$,replacement;^both$,replacement,is an Annex K function'}}"\ +// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 + +typedef __SIZE_TYPE__ size_t; +typedef __WCHAR_TYPE__ wchar_t; + +void non_annex_k_only(void); +void annex_k_only(void); +void both(void); + +void f1() { + non_annex_k_only(); + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead + // no-warning WITH-ONLY-ANNEX-K + annex_k_only(); + // no-warning WITHOUT-ANNEX-K + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-2]]:3: warning: function 'annex_k_only' is marked as unsafe; 'replacement' should be used instead + // CHECK-MESSAGES-WITH-ONLY-ANNEX-K: :[[@LINE-3]]:3: warning: function 'annex_k_only' is marked as unsafe; 'replacement' should be used instead + both(); + // CHECK-MESSAGES-WITH-ANNEX-K: :[[@LINE-1]]:3: warning: function 'both' is an Annex K function; 'replacement' should be used instead + // CHECK-MESSAGES-WITHOUT-ANNEX-K: :[[@LINE-2]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead + // CHECK-MESSAGES-WITH-ONLY-ANNEX-K: :[[@LINE-3]]:3: warning: function 'both' is an Annex K function; 'replacement' should be used instead +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c index 4bc2bad996d706..0409dd6bfcaa3d 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions.c @@ -12,6 +12,12 @@ // RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K-CERT-ONLY %s bugprone-unsafe-functions %t -- \ // RUN: -config="{CheckOptions: {bugprone-unsafe-functions.ReportMoreUnsafeFunctions: false}}" \ // RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 +// RUN: %check_clang_tidy -check-suffix=WITH-NONE-ENABLED %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.ReportDefaultFunctions: false}}" \ +// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 + +// CHECK-MESSAGES-WITH-NONE-ENABLED: 1 warning generated +// CHECK-MESSAGES-WITH-NONE-ENABLED: Suppressed 1 warnings typedef __SIZE_TYPE__ size_t; typedef __WCHAR_TYPE__ wchar_t; >From 47c1a5694fed5769bb6bbc437e31276efe41d520 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 28 Aug 2024 08:49:46 +0000 Subject: [PATCH 2/3] [clang-tidy] Make MatchesAnyListedNameMatcher::NameMatcher public The same matching mechanism should be used both there and within bugprone-unsafe-functions check. --- clang-tools-extra/clang-tidy/utils/Matchers.h | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/clang-tools-extra/clang-tidy/utils/Matchers.h b/clang-tools-extra/clang-tidy/utils/Matchers.h index 5fd98db9678708..451c4ce92585b9 100644 --- a/clang-tools-extra/clang-tidy/utils/Matchers.h +++ b/clang-tools-extra/clang-tidy/utils/Matchers.h @@ -85,15 +85,7 @@ class MatchesAnyListedNameMatcher NameList.begin(), NameList.end(), std::back_inserter(NameMatchers), [](const llvm::StringRef Name) { return NameMatcher(Name); }); } - bool matches( - const NamedDecl &Node, ast_matchers::internal::ASTMatchFinder *Finder, - ast_matchers::internal::BoundNodesTreeBuilder *Builder) const override { - return llvm::any_of(NameMatchers, [&Node](const NameMatcher &NM) { - return NM.match(Node); - }); - } -private: class NameMatcher { llvm::Regex Regex; enum class MatchMode { @@ -136,6 +128,15 @@ class MatchesAnyListedNameMatcher } }; + bool matches( + const NamedDecl &Node, ast_matchers::internal::ASTMatchFinder *Finder, + ast_matchers::internal::BoundNodesTreeBuilder *Builder) const override { + return llvm::any_of(NameMatchers, [&Node](const NameMatcher &NM) { + return NM.match(Node); + }); + } + +private: std::vector<NameMatcher> NameMatchers; }; >From 476d571c5472336ae589f05c919f3551a5024e18 Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 28 Aug 2024 08:51:59 +0000 Subject: [PATCH 3/3] [clang-tidy] Add regex support and documentation for C++ functions for bugprone-unsafe-functions check --- .../bugprone/UnsafeFunctionsCheck.cpp | 11 ++-- .../bugprone/UnsafeFunctionsCheck.h | 3 +- .../checks/bugprone/unsafe-functions.rst | 4 ++ .../unsafe-functions-custom-regex.cpp | 63 +++++++++++++++++++ .../bugprone/unsafe-functions-custom.c | 3 - 5 files changed, 76 insertions(+), 8 deletions(-) create mode 100644 clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp index 05c2063402b080..c2f23c309f1b54 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -167,8 +167,10 @@ ParseCheckedFunctions(StringRef Option, StringRef OptionName, << Name.trim() << OptionName; } - Result.push_back({Name.trim().str(), llvm::Regex(Name.trim()), - Replacement.trim().str(), Reason.trim().str()}); + Result.push_back( + {Name.trim().str(), + matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()), + Replacement.trim().str(), Reason.trim().str()}); } return Result; @@ -324,7 +326,7 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { if (AnnexKIsAvailable) { for (const auto &Entry : CustomAnnexKFunctions) { - if (Entry.Pattern.match(FunctionName)) { + if (Entry.Pattern.match(*FuncDecl)) { // If both Annex K and Normal are matched, show Annex K warning only. if (CustomAnnexK) ShowCheckedFunctionWarning(Entry); @@ -341,7 +343,8 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) { return; for (const auto &Entry : CustomNormalFunctions) { - if (Entry.Pattern.match(FunctionName)) { + + if (Entry.Pattern.match(*FuncDecl)) { ShowCheckedFunctionWarning(Entry); return; } diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h index eb2656096256ba..ad23a807666e79 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h @@ -10,6 +10,7 @@ #define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_UNSAFEFUNCTIONSCHECK_H #include "../ClangTidyCheck.h" +#include "../utils/Matchers.h" #include <optional> namespace clang::tidy::bugprone { @@ -34,7 +35,7 @@ class UnsafeFunctionsCheck : public ClangTidyCheck { struct CheckedFunction { std::string Name; - llvm::Regex Pattern; + matchers::MatchesAnyListedNameMatcher::NameMatcher Pattern; std::string Replacement; std::string Reason; }; diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst index 4402eba2647978..e97794cd8f30b0 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst @@ -107,6 +107,10 @@ will produce the following diagnostic message. ::std::original(); // no-warning original_function(); // no-warning +If the regular expression contains the character ``:``, it is matched against the +qualified name (i.e. ``std::original``), otherwise the regex is matched against the unqualified name (``original``). +If the regular expression starts with ``::`` (or ``^::``), it is matched against the +fully qualified name (``::std::original``). Options ------- diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp new file mode 100644 index 00000000000000..4554c43fb700d5 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp @@ -0,0 +1,63 @@ +// This test fails on "x86_64-sie" buildbot and "x86_64-scei-ps4" target. +// According to @dyung, something related to the kind of standard library +// availability is causing the failure. Even though we explicitly define +// the relevant macros the check is hunting for in the invocation, the real +// parsing and preprocessor state will not have that case. +// UNSUPPORTED: target={{.*-(ps4|ps5)}} +// +// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomNormalFunctions: '::non_annex_k_only,replacement;::both,replacement,is a normal function'}}"\ +// RUN: -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomNormalFunctions: '^non_annex_k_only$,replacement,is matched on function name only;^::both,replacement,is matched on qualname prefix'}}"\ +// RUN: -- -U__STDC_LIB_EXT1__ -U__STDC_WANT_LIB_EXT1__ +// RUN: %check_clang_tidy -check-suffix=WITH-ANNEX-K %s bugprone-unsafe-functions %t --\ +// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomAnnexKFunctions: '::annex_k_only,replacement;::both,replacement,is an Annex K function'}}"\ +// RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 + +// TODO: How is "Annex K" available in C++ mode? +// no-warning WITH-ANNEX-K + +void non_annex_k_only(); +void annex_k_only(); +void both(); + +namespace regex_test { +void non_annex_k_only(); +void both(); +} + +void non_annex_k_only_regex(); +void both_regex(); + +void f1() { + non_annex_k_only(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is matched on function name only; 'replacement' should be used instead + annex_k_only(); + // no-warning NON-STRICT-REGEX + // no-warning STRICT-REGEX + both(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'both' is matched on qualname prefix; 'replacement' should be used instead + + ::non_annex_k_only(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is matched on function name only; 'replacement' should be used instead + regex_test::non_annex_k_only(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only' is marked as unsafe; 'replacement' should be used instead + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'non_annex_k_only' is matched on function name only; 'replacement' should be used instead + non_annex_k_only_regex(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'non_annex_k_only_regex' is marked as unsafe; 'replacement' should be used instead + // no-warning STRICT-REGEX + + ::both(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'both' is matched on qualname prefix; 'replacement' should be used instead + regex_test::both(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both' is a normal function; 'replacement' should be used instead + // no-warning STRICT-REGEX + both_regex(); + // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'both_regex' is a normal function; 'replacement' should be used instead + // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'both_regex' is matched on qualname prefix; 'replacement' should be used instead +} diff --git a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c index ab70970bf749c3..69acb381b9d4ed 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c +++ b/clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom.c @@ -15,9 +15,6 @@ // RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomAnnexKFunctions: '^annex_k_only$,replacement;^both$,replacement,is an Annex K function'}}"\ // RUN: -- -D__STDC_LIB_EXT1__=1 -D__STDC_WANT_LIB_EXT1__=1 -typedef __SIZE_TYPE__ size_t; -typedef __WCHAR_TYPE__ wchar_t; - void non_annex_k_only(void); void annex_k_only(void); void both(void); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits