[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
Tue Sep 17 01:41:56 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 01/10] [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 02/10] [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 03/10] [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);
>From a11b0b7bd055a39571544b76d3073dc8ed08602a Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Mon, 2 Sep 2024 08:13:08 +0000
Subject: [PATCH 04/10] Review fixes
Do not add invalid options to the functions list
Rewrite description for options
Minor formatting fixes
---
.../bugprone/UnsafeFunctionsCheck.cpp | 17 +++++-----
clang-tools-extra/docs/ReleaseNotes.rst | 4 +++
.../checks/bugprone/unsafe-functions.rst | 31 ++++++++++++-------
3 files changed, 33 insertions(+), 19 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index c2f23c309f1b54..0e2fd7c53db057 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -140,7 +140,7 @@ static bool isAnnexKAvailable(std::optional<bool> &CacheVar, Preprocessor *PP,
}
static std::vector<UnsafeFunctionsCheck::CheckedFunction>
-ParseCheckedFunctions(StringRef Option, StringRef OptionName,
+parseCheckedFunctions(StringRef Option, StringRef OptionName,
ClangTidyContext *Context) {
std::vector<StringRef> Functions = utils::options::parseStringList(Option);
std::vector<UnsafeFunctionsCheck::CheckedFunction> Result;
@@ -158,6 +158,7 @@ ParseCheckedFunctions(StringRef Option, StringRef OptionName,
Context->configurationDiag("invalid configuration value for option '%0'; "
"expected the name of an unsafe function")
<< OptionName;
+ continue;
}
if (Replacement.trim().empty()) {
@@ -165,6 +166,7 @@ ParseCheckedFunctions(StringRef Option, StringRef OptionName,
"invalid configuration value '%0' for option '%1'; "
"expected a replacement function name")
<< Name.trim() << OptionName;
+ continue;
}
Result.push_back(
@@ -176,7 +178,7 @@ ParseCheckedFunctions(StringRef Option, StringRef OptionName,
return Result;
}
-static std::string SerializeCheckedFunctions(
+static std::string serializeCheckedFunctions(
const std::vector<UnsafeFunctionsCheck::CheckedFunction> &Functions) {
std::vector<std::string> Result;
Result.reserve(Functions.size());
@@ -195,10 +197,10 @@ static std::string SerializeCheckedFunctions(
UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- CustomNormalFunctions(ParseCheckedFunctions(
+ CustomNormalFunctions(parseCheckedFunctions(
Options.get(OptionNameCustomNormalFunctions, ""),
OptionNameCustomNormalFunctions, Context)),
- CustomAnnexKFunctions(ParseCheckedFunctions(
+ CustomAnnexKFunctions(parseCheckedFunctions(
Options.get(OptionNameCustomAnnexKFunctions, ""),
OptionNameCustomAnnexKFunctions, Context)),
ReportDefaultFunctions(
@@ -208,9 +210,9 @@ UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name,
void UnsafeFunctionsCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, OptionNameCustomNormalFunctions,
- SerializeCheckedFunctions(CustomNormalFunctions));
+ serializeCheckedFunctions(CustomNormalFunctions));
Options.store(Opts, OptionNameCustomAnnexKFunctions,
- SerializeCheckedFunctions(CustomAnnexKFunctions));
+ serializeCheckedFunctions(CustomAnnexKFunctions));
Options.store(Opts, OptionNameReportDefaultFunctions, ReportDefaultFunctions);
Options.store(Opts, OptionNameReportMoreUnsafeFunctions,
ReportMoreUnsafeFunctions);
@@ -343,14 +345,13 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
return;
for (const auto &Entry : CustomNormalFunctions) {
-
if (Entry.Pattern.match(*FuncDecl)) {
ShowCheckedFunctionWarning(Entry);
return;
}
}
- assert(false && "No custom function was matched.");
+ llvm_unreachable("No custom function was matched.");
return;
}
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index b001a6ad446695..d4ce7f1e80a8ce 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -104,6 +104,10 @@ New check aliases
Changes in existing checks
^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Improved :doc:`bugprone-unsafe-functions
+ <clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
+ custom matched functions as well.
+
- Improved :doc:`modernize-use-std-format
<clang-tidy/checks/modernize/use-std-format>` check to support replacing
member function calls too.
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 e97794cd8f30b0..cfc9a357033822 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
@@ -20,7 +20,7 @@ Unsafe functions
----------------
The following functions are reported if `ReportDefaultFunctions
-<unsafe-functions.html#cmdoption-arg-ReportDefaultFunctions>`_ is enabled.
+<unsafe-functions.html#option-ReportDefaultFunctions>`_ is enabled.
If *Annex K.* is available, a replacement from *Annex K.* is suggested for the
following functions:
@@ -49,7 +49,7 @@ The following functions are always checked, regardless of *Annex K* availability
- ``setbuf``, suggested replacement: ``setvbuf``
If `ReportMoreUnsafeFunctions
-<unsafe-functions.html#cmdoption-arg-ReportMoreUnsafeFunctions>`_ is enabled,
+<unsafe-functions.html#option-ReportMoreUnsafeFunctions>`_ is enabled,
the following functions are also checked:
- ``bcmp``, suggested replacement: ``memcmp``
@@ -77,6 +77,8 @@ 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:
+
Custom functions
----------------
@@ -96,9 +98,9 @@ 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``.
+reasoning behind the replacement. The default reason is `is marked as unsafe`.
-As an example, the configuration ``^original$, replacement, is deprecated;``
+As an example, the configuration `^original$, replacement, is deprecated;`
will produce the following diagnostic message.
.. code:: c
@@ -107,14 +109,16 @@ 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
+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
+If the regular expression starts with `::` (or `^::`), it is matched against the
fully qualified name (``::std::original``).
Options
-------
+.. _option-ReportMoreUnsafeFunctions:
+
.. option:: ReportMoreUnsafeFunctions
When `true`, additional functions from widely used APIs (such as POSIX) are
@@ -123,6 +127,8 @@ Options
this option enables.
Default is `true`.
+.. _option-ReportDefaultFunctions:
+
.. option:: ReportDefaultFunctions
When `true`, the check reports the default set of functions.
@@ -130,14 +136,17 @@ Options
.. 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>`_.
+ A semicolon-separated list of custom functions to be matched. A matched
+ function contains a regular expression, the name of the replacement
+ function, and an optional reason, separated by comma. 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
+ A semicolon-separated list of custom functions to be matched, if Annex K is
+ available. A matched function contains a regular expression, the name of the
+ replacement function, and an optional reason, separated by comma. For more
+ information, see `Custom functions
<unsafe-functions.html#custom-functions>`_.
Examples
>From ca277a904711f5a9702205ef66f810e2c488af77 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Mon, 2 Sep 2024 08:35:43 +0000
Subject: [PATCH 05/10] Remove redundant import
---
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index 0e2fd7c53db057..cbd16bd2b8cf12 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -7,7 +7,6 @@
//===----------------------------------------------------------------------===//
#include "UnsafeFunctionsCheck.h"
-#include "../utils/Matchers.h"
#include "../utils/OptionsUtils.h"
#include "clang/AST/ASTContext.h"
#include "clang/ASTMatchers/ASTMatchFinder.h"
>From d65297e22992b78b190dff48f24338febe334330 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Wed, 11 Sep 2024 08:14:29 +0000
Subject: [PATCH 06/10] Remove CustomAnnexKFunctions and related logic
Ultimately it would not get much use outside of being able to reproduce
the default check logic.
---
.../bugprone/UnsafeFunctionsCheck.cpp | 95 +++++--------------
.../bugprone/UnsafeFunctionsCheck.h | 3 +-
.../unsafe-functions-custom-regex.cpp | 79 ++++++---------
.../bugprone/unsafe-functions-custom.c | 54 +++++------
4 files changed, 77 insertions(+), 154 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index cbd16bd2b8cf12..14e8f73bf246fc 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -19,10 +19,8 @@ using namespace llvm;
namespace clang::tidy::bugprone {
-static constexpr llvm::StringLiteral OptionNameCustomNormalFunctions =
- "CustomNormalFunctions";
-static constexpr llvm::StringLiteral OptionNameCustomAnnexKFunctions =
- "CustomAnnexKFunctions";
+static constexpr llvm::StringLiteral OptionNameCustomFunctions =
+ "CustomFunctions";
static constexpr llvm::StringLiteral OptionNameReportDefaultFunctions =
"ReportDefaultFunctions";
static constexpr llvm::StringLiteral OptionNameReportMoreUnsafeFunctions =
@@ -35,8 +33,6 @@ 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>
@@ -139,8 +135,7 @@ static bool isAnnexKAvailable(std::optional<bool> &CacheVar, Preprocessor *PP,
}
static std::vector<UnsafeFunctionsCheck::CheckedFunction>
-parseCheckedFunctions(StringRef Option, StringRef OptionName,
- ClangTidyContext *Context) {
+parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) {
std::vector<StringRef> Functions = utils::options::parseStringList(Option);
std::vector<UnsafeFunctionsCheck::CheckedFunction> Result;
Result.reserve(Functions.size());
@@ -156,7 +151,7 @@ parseCheckedFunctions(StringRef Option, StringRef OptionName,
if (Name.trim().empty()) {
Context->configurationDiag("invalid configuration value for option '%0'; "
"expected the name of an unsafe function")
- << OptionName;
+ << OptionNameCustomFunctions;
continue;
}
@@ -164,7 +159,7 @@ parseCheckedFunctions(StringRef Option, StringRef OptionName,
Context->configurationDiag(
"invalid configuration value '%0' for option '%1'; "
"expected a replacement function name")
- << Name.trim() << OptionName;
+ << Name.trim() << OptionNameCustomFunctions;
continue;
}
@@ -196,22 +191,16 @@ static std::string serializeCheckedFunctions(
UnsafeFunctionsCheck::UnsafeFunctionsCheck(StringRef Name,
ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
- CustomNormalFunctions(parseCheckedFunctions(
- Options.get(OptionNameCustomNormalFunctions, ""),
- OptionNameCustomNormalFunctions, Context)),
- CustomAnnexKFunctions(parseCheckedFunctions(
- Options.get(OptionNameCustomAnnexKFunctions, ""),
- OptionNameCustomAnnexKFunctions, Context)),
+ CustomFunctions(parseCheckedFunctions(
+ Options.get(OptionNameCustomFunctions, ""), 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, OptionNameCustomFunctions,
+ serializeCheckedFunctions(CustomFunctions));
Options.store(Opts, OptionNameReportDefaultFunctions, ReportDefaultFunctions);
Options.store(Opts, OptionNameReportMoreUnsafeFunctions,
ReportMoreUnsafeFunctions);
@@ -262,27 +251,11 @@ void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) {
}
}
- if (!CustomAnnexKFunctions.empty()) {
+ if (!CustomFunctions.empty()) {
std::vector<llvm::StringRef> FunctionNames;
- FunctionNames.reserve(CustomAnnexKFunctions.size());
+ FunctionNames.reserve(CustomFunctions.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)
+ for (const auto &Entry : CustomFunctions)
FunctionNames.push_back(Entry.Name);
auto CustomFunctionsMatcher = matchers::matchesAnyListedName(FunctionNames);
@@ -305,47 +278,25 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
const auto *Normal = Result.Nodes.getNodeAs<FunctionDecl>(FunctionNamesId);
const auto *Additional =
Result.Nodes.getNodeAs<FunctionDecl>(AdditionalFunctionNamesId);
- const auto *CustomAnnexK =
- Result.Nodes.getNodeAs<FunctionDecl>(CustomAnnexKFunctionNamesId);
- const auto *CustomNormal =
+ const auto *Custom =
Result.Nodes.getNodeAs<FunctionDecl>(CustomFunctionNamesId);
- assert((AnnexK || Normal || Additional || CustomAnnexK || CustomNormal) &&
+ assert((AnnexK || Normal || Additional || Custom) &&
"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(*FuncDecl)) {
- // 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 (Custom) {
+ for (const auto &Entry : CustomFunctions) {
if (Entry.Pattern.match(*FuncDecl)) {
- ShowCheckedFunctionWarning(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();
return;
}
}
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
index ad23a807666e79..63058c326ef291 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.h
@@ -41,8 +41,7 @@ class UnsafeFunctionsCheck : public ClangTidyCheck {
};
private:
- const std::vector<CheckedFunction> CustomNormalFunctions;
- const std::vector<CheckedFunction> CustomAnnexKFunctions;
+ const std::vector<CheckedFunction> CustomFunctions;
// If true, the default set of functions are reported.
const bool ReportDefaultFunctions;
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
index 4554c43fb700d5..f475e7093e3a87 100644
--- 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
@@ -1,63 +1,44 @@
-// 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: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,replacement,is matched on qualname prefix'}}"
// 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
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,replacement,is a full qualname match'}}"
-// 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();
+void name_match();
+void prefix_match();
namespace regex_test {
-void non_annex_k_only();
-void both();
+void name_match();
+void prefix_match();
}
-void non_annex_k_only_regex();
-void both_regex();
+void name_match_regex();
+void prefix_match_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
+ name_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
+ prefix_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; '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
+ ::name_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
+ regex_test::name_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
+ name_match_regex();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match; '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
+ ::prefix_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; 'replacement' should be used instead
+ regex_test::prefix_match();
+ // no-warning NON-STRICT-REGEX
+ // no-warning STRICT-REGEX
+ prefix_match_regex();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; '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 69acb381b9d4ed..dacd2d5bb87006 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
@@ -1,35 +1,27 @@
-// 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
+// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,replacement,is matched on qualname prefix'}}"
+// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,replacement,is a full qualname match'}}"
-void non_annex_k_only(void);
-void annex_k_only(void);
-void both(void);
+void name_match();
+void prefix_match();
+
+void name_match_regex();
+void prefix_match_regex();
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
+ name_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
+ prefix_match();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; 'replacement' should be used instead
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; 'replacement' should be used instead
+
+ name_match_regex();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match; 'replacement' should be used instead
+ // no-warning STRICT-REGEX
+
+ prefix_match_regex();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; 'replacement' should be used instead
+ // no-warning STRICT-REGEX
}
>From 3fdf79f589a0b6d1671e75383c7810a654ab0856 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Wed, 11 Sep 2024 08:16:49 +0000
Subject: [PATCH 07/10] Remove CustomAnnexKFunctions from docs
---
.../checks/bugprone/unsafe-functions.rst | 17 ++++-------------
1 file changed, 4 insertions(+), 13 deletions(-)
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 cfc9a357033822..457a973346ec18 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
@@ -82,13 +82,12 @@ 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:
+The option `CustomFunctions` allows the user to define custom functions to be
+checked. The format is the following, without newlines:
.. code::
- bugprone-unsafe-functions.CustomNormalFunctions="
+ bugprone-unsafe-functions.CustomFunctions="
functionRegex1, replacement1[, reason1];
functionRegex2, replacement2[, reason2];
...
@@ -134,21 +133,13 @@ Options
When `true`, the check reports the default set of functions.
Default is `true`.
-.. option:: CustomNormalFunctions
+.. option:: CustomFunctions
A semicolon-separated list of custom functions to be matched. A matched
function contains a regular expression, the name of the replacement
function, and an optional reason, separated by comma. For more information,
see `Custom functions <unsafe-functions.html#custom-functions>`_.
-.. option:: CustomAnnexKFunctions
-
- A semicolon-separated list of custom functions to be matched, if Annex K is
- available. A matched function contains a regular expression, the name of the
- replacement function, and an optional reason, separated by comma. For more
- information, see `Custom functions
- <unsafe-functions.html#custom-functions>`_.
-
Examples
--------
>From b52c54b55fe3047ace114a3bf760e29bc06dff77 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Wed, 11 Sep 2024 08:23:30 +0000
Subject: [PATCH 08/10] Attempt to fix rst references (again)
---
.../clang-tidy/checks/bugprone/unsafe-functions.rst | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
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 457a973346ec18..6865cb82232ecf 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,8 +19,7 @@ The check implements the following rules from the CERT C Coding Standard:
Unsafe functions
----------------
-The following functions are reported if `ReportDefaultFunctions
-<unsafe-functions.html#option-ReportDefaultFunctions>`_ is enabled.
+The following functions are reported if :ref:`ReportDefaultFunctions<option-ReportDefaultFunctions>` is enabled.
If *Annex K.* is available, a replacement from *Annex K.* is suggested for the
following functions:
@@ -48,8 +47,7 @@ The following functions are always checked, regardless of *Annex K* availability
- ``rewind``, suggested replacement: ``fseek``
- ``setbuf``, suggested replacement: ``setvbuf``
-If `ReportMoreUnsafeFunctions
-<unsafe-functions.html#option-ReportMoreUnsafeFunctions>`_ is enabled,
+If :ref:`ReportMoreUnsafeFunctions<option-ReportMoreUnsafeFunctions>` is enabled,
the following functions are also checked:
- ``bcmp``, suggested replacement: ``memcmp``
@@ -77,7 +75,7 @@ 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:
+.. _CustomFunctions:
Custom functions
----------------
@@ -138,7 +136,7 @@ Options
A semicolon-separated list of custom functions to be matched. A matched
function contains a regular expression, the name of the replacement
function, and an optional reason, separated by comma. For more information,
- see `Custom functions <unsafe-functions.html#custom-functions>`_.
+ see :ref:`Custom functions<CustomFunctions>`.
Examples
--------
>From 91297f5956443789132c80b4c27034faed7ea956 Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Wed, 11 Sep 2024 09:00:12 +0000
Subject: [PATCH 09/10] Small constness fix
---
clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index 14e8f73bf246fc..a9a203d9693204 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -136,7 +136,8 @@ static bool isAnnexKAvailable(std::optional<bool> &CacheVar, Preprocessor *PP,
static std::vector<UnsafeFunctionsCheck::CheckedFunction>
parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) {
- std::vector<StringRef> Functions = utils::options::parseStringList(Option);
+ const std::vector<StringRef> Functions =
+ utils::options::parseStringList(Option);
std::vector<UnsafeFunctionsCheck::CheckedFunction> Result;
Result.reserve(Functions.size());
>From 6ff0347178170dbed6429c127610b1ef7f90df9c Mon Sep 17 00:00:00 2001
From: Viktor <viktor.cseh at ericsson.com>
Date: Tue, 17 Sep 2024 08:41:30 +0000
Subject: [PATCH 10/10] Add support for no specified replacement function
---
.../bugprone/UnsafeFunctionsCheck.cpp | 23 +++++++++----------
.../checks/bugprone/unsafe-functions.rst | 9 +++++---
.../unsafe-functions-custom-regex.cpp | 14 +++++------
.../bugprone/unsafe-functions-custom.c | 10 ++++----
4 files changed, 29 insertions(+), 27 deletions(-)
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index a9a203d9693204..10780d7a551a1d 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -156,14 +156,6 @@ parseCheckedFunctions(StringRef Option, ClangTidyContext *Context) {
continue;
}
- if (Replacement.trim().empty()) {
- Context->configurationDiag(
- "invalid configuration value '%0' for option '%1'; "
- "expected a replacement function name")
- << Name.trim() << OptionNameCustomFunctions;
- continue;
- }
-
Result.push_back(
{Name.trim().str(),
matchers::MatchesAnyListedNameMatcher::NameMatcher(Name.trim()),
@@ -294,10 +286,17 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
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 (Entry.Replacement.empty()) {
+ diag(DeclRef->getExprLoc(), "function %0 %1; it should not be used")
+ << FuncDecl << Reason << Entry.Replacement
+ << DeclRef->getSourceRange();
+ } else {
+ diag(DeclRef->getExprLoc(),
+ "function %0 %1; '%2' should be used instead")
+ << FuncDecl << Reason << Entry.Replacement
+ << DeclRef->getSourceRange();
+ }
+
return;
}
}
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 6865cb82232ecf..a8680c72b85ac5 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
@@ -86,8 +86,8 @@ checked. The format is the following, without newlines:
.. code::
bugprone-unsafe-functions.CustomFunctions="
- functionRegex1, replacement1[, reason1];
- functionRegex2, replacement2[, reason2];
+ functionRegex1[, replacement1[, reason1]];
+ functionRegex2[, replacement2[, reason2]];
...
"
@@ -97,6 +97,9 @@ The functions are matched using POSIX extended regular expressions.
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`.
+If `replacement` is empty, the text `it should not be used` will be shown
+instead of the suggestion for a replacement.
+
As an example, the configuration `^original$, replacement, is deprecated;`
will produce the following diagnostic message.
@@ -134,7 +137,7 @@ Options
.. option:: CustomFunctions
A semicolon-separated list of custom functions to be matched. A matched
- function contains a regular expression, the name of the replacement
+ function contains a regular expression, an optional name of the replacement
function, and an optional reason, separated by comma. For more information,
see :ref:`Custom functions<CustomFunctions>`.
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
index f475e7093e3a87..fc97d1bc93bc54 100644
--- 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
@@ -1,7 +1,7 @@
// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\
-// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,replacement,is matched on qualname prefix'}}"
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix'}}"
// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\
-// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,replacement,is a full qualname match'}}"
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match'}}"
void name_match();
void prefix_match();
@@ -19,8 +19,8 @@ void f1() {
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
prefix_match();
- // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; 'replacement' should be used instead
- // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used
::name_match();
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
@@ -33,12 +33,12 @@ void f1() {
// no-warning STRICT-REGEX
::prefix_match();
- // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; 'replacement' should be used instead
- // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used
regex_test::prefix_match();
// no-warning NON-STRICT-REGEX
// no-warning STRICT-REGEX
prefix_match_regex();
- // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; 'replacement' should be used instead
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used
// no-warning STRICT-REGEX
}
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 dacd2d5bb87006..7fd71ec2f2e7b0 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
@@ -1,7 +1,7 @@
// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\
-// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,replacement,is matched on qualname prefix'}}"
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix'}}"
// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\
-// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,replacement,is a full qualname match'}}"
+// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match'}}"
void name_match();
void prefix_match();
@@ -14,14 +14,14 @@ void f1() {
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match' is a qualname match; 'replacement' should be used instead
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'name_match' is matched on function name only; 'replacement' should be used instead
prefix_match();
- // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; 'replacement' should be used instead
- // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; 'replacement' should be used instead
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match' is matched on qualname prefix; it should not be used
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'prefix_match' is a full qualname match; it should not be used
name_match_regex();
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'name_match_regex' is a qualname match; 'replacement' should be used instead
// no-warning STRICT-REGEX
prefix_match_regex();
- // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; 'replacement' should be used instead
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used
// no-warning STRICT-REGEX
}
More information about the cfe-commits
mailing list