https://github.com/Discookie created https://github.com/llvm/llvm-project/pull/106350
Adds the check options `bugprone-unsafe-functions.CustomNormalFunctions` and `CustomAnnexKFunctions` to be able to match user-defined functions as part of the checker. Adds the option `bugprone-unsafe-functions.ReportDefaultFunctions` to disable reporting the default set of functions as well. The functions names are matched using the same mechanism as the `matchesAnyListedName` tidy matcher, documented in the [check docs](https://github.com/Discookie/llvm-project/blob/unsafe-expand/clang-tools-extra/docs/clang-tidy/checks/bugprone/unsafe-functions.rst). >From 4cdba3f6c77aa9af5f9e248c6ad8aa2de89eac0e Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Thu, 15 Aug 2024 08:41:00 +0000 Subject: [PATCH 1/3] [clang-tidy] Add user-defined functions to bugprone-unsafe-functions check --- .../bugprone/UnsafeFunctionsCheck.cpp | 215 +++++++++++++++--- .../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, 288 insertions(+), 32 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..ac98ebaebed83b 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,8 @@ 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,55 +137,151 @@ 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"); + 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(AdditionalFunctionNamesMatcher) - .bind(AdditionalFunctionNamesId))) + 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 +292,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 dd734d95cc3ac6ce9fc9cffeb6b9d6e540f59eae Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Tue, 27 Aug 2024 10:22:36 +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 | 18 ++++++++++-------- 1 file changed, 10 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..f79d872300320b 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,16 @@ 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 80694ad203d0d37cd8d186d823bb536c921ae9bf Mon Sep 17 00:00:00 2001 From: Viktor <viktor.c...@ericsson.com> Date: Wed, 28 Aug 2024 08:30:17 +0000 Subject: [PATCH 3/3] [clang-tidy] Add regex support and documentation for C++ functions for bugprone-unsafe-functions check --- .../bugprone/UnsafeFunctionsCheck.cpp | 7 ++- .../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, 73 insertions(+), 7 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 ac98ebaebed83b..8d9ffda04a847a 100644 --- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp +++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp @@ -164,7 +164,7 @@ ParseCheckedFunctions(StringRef Option, StringRef OptionName, "expected a replacement function name") << Name.trim() << OptionName; } - Result.push_back({ Name.trim().str(), llvm::Regex(Name.trim()), + Result.push_back({ Name.trim().str(), matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()), Replacement.trim().str(), Reason.trim().str() }); } @@ -320,7 +320,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); @@ -337,7 +337,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
