[clang-tools-extra] [clang-tidy] Add user-defined functions to `bugprone-unsafe-functions` check (PR #106350)

via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 28 01:53:23 PDT 2024


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.cseh at 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.cseh at 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.cseh at 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);



More information about the cfe-commits mailing list