[clang-tools-extra] [clang-tidy] Add C++ member function support to custom bugprone-unsafe-functions matches (PR #117165)
via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 21 06:23:34 PST 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tidy
Author: Discookie (Discookie)
<details>
<summary>Changes</summary>
Before, C++ member functions in the format of ``Class instance; instance.memberfn();`` were unable to be matched.
This PR adds support for this type of call, and it is matched in exactly the same format as other functions.
---
Full diff: https://github.com/llvm/llvm-project/pull/117165.diff
4 Files Affected:
- (modified) clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp (+29-9)
- (modified) clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp (+157)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+1-1)
- (modified) clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp (+23-2)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
index 604a7cac0e4903..a45949314a4ca8 100644
--- a/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp
@@ -256,13 +256,32 @@ void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) {
.bind(CustomFunctionNamesId)))
.bind(DeclRefId),
this);
+ // C++ member calls do not contain a DeclRefExpr to the function decl.
+ // Instead, they contain a MemberExpr that refers to the decl.
+ Finder->addMatcher(memberExpr(member(functionDecl(CustomFunctionsMatcher)
+ .bind(CustomFunctionNamesId)))
+ .bind(DeclRefId),
+ this);
}
}
void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
- const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId);
- const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
- assert(DeclRef && FuncDecl && "No valid matched node in check()");
+ const Expr *SourceExpr;
+ const FunctionDecl *FuncDecl;
+
+ if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId)) {
+ SourceExpr = DeclRef;
+ FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
+ } else if (const auto *Member =
+ Result.Nodes.getNodeAs<MemberExpr>(DeclRefId)) {
+ SourceExpr = Member;
+ FuncDecl = cast<FunctionDecl>(Member->getMemberDecl());
+ } else {
+ llvm_unreachable("No valid matched node in check()");
+ return;
+ }
+
+ assert(SourceExpr && FuncDecl && "No valid matched node in check()");
// Only one of these are matched at a time.
const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>(
@@ -286,14 +305,15 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str();
if (Entry.Replacement.empty()) {
- diag(DeclRef->getExprLoc(), "function %0 %1; it should not be used")
+ diag(SourceExpr->getExprLoc(),
+ "function %0 %1; it should not be used")
<< FuncDecl << Reason << Entry.Replacement
- << DeclRef->getSourceRange();
+ << SourceExpr->getSourceRange();
} else {
- diag(DeclRef->getExprLoc(),
+ diag(SourceExpr->getExprLoc(),
"function %0 %1; '%2' should be used instead")
<< FuncDecl << Reason << Entry.Replacement
- << DeclRef->getSourceRange();
+ << SourceExpr->getSourceRange();
}
return;
@@ -323,9 +343,9 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
if (!ReplacementFunctionName)
return;
- diag(DeclRef->getExprLoc(), "function %0 %1; '%2' should be used instead")
+ diag(SourceExpr->getExprLoc(), "function %0 %1; '%2' should be used instead")
<< FuncDecl << getRationaleFor(FunctionName)
- << ReplacementFunctionName.value() << DeclRef->getSourceRange();
+ << ReplacementFunctionName.value() << SourceExpr->getSourceRange();
}
void UnsafeFunctionsCheck::registerPPCallbacks(
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 26befe0de59ae4..0bd9c109af97f5 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -331,6 +331,10 @@ class CERTModule : public ClangTidyModule {
// STR
CheckFactories.registerCheck<bugprone::SignedCharMisuseCheck>(
"cert-str34-c");
+ // temp
+
+ CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
+ "ericsson-unsafe-functions");
}
ClangTidyOptions getModuleOptions() override {
@@ -347,6 +351,159 @@ class CERTModule : public ClangTidyModule {
Opts["cert-err33-c.AllowCastToVoid"] = "true";
Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "false";
Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "false";
+ Opts["ericsson-unsafe-functions.ReportDefaultFunctions"] = "false";
+ Opts["ericsson-unsafe-functions.CustomFunctions"] =
+ // High priority
+ "^::gets$,fgets,is insecure and deprecated;"
+ "^::sprintf$,snprintf,is insecure and deprecated;"
+ "^::strcpy$,strlcpy,is insecure and deprecated;"
+ "^::strncpy$,strlcpy,is insecure and deprecated;"
+ "^::vsprintf,vsnprintf,is insecure and deprecated;"
+ "^::wcscat$,wcslcat,is insecure and deprecated;"
+ "^::wcscpy$,wcslcpy,is insecure and deprecated;"
+ "^::wcsncat$,wcslcat,is insecure and deprecated;"
+ "^::wcsncpy$,wcslcpy,is insecure and deprecated;"
+
+ "^::bcopy$,memcpy or memmove,is insecure and deprecated;"
+ "^::bzero$,memset,is insecure and deprecated;"
+ "^::index$,strchr,is insecure and deprecated;"
+ "^::rindex$,strrchr,is insecure and deprecated;"
+
+ "^::valloc$,aligned_alloc,is insecure and deprecated;"
+
+ "^::tmpnam$,mkstemp,is insecure and deprecated;"
+ "^::tmpnam_r$,mkstemp,is insecure and deprecated;"
+
+ "^::getwd$,getcwd,is insecure and deprecated;"
+ "^::crypt$,an Ericsson-recommended crypto algorithm,is insecure and "
+ "deprecated;"
+ "^::encrypt$,an Ericsson-recommended crypto algorithm,is insecure and "
+ "deprecated;"
+ "^::stpcpy$,strlcpy,is insecure and deprecated;"
+ "^::strcat$,strlcat,is insecure and deprecated;"
+ "^::strncat$,strlcat,is insecure and deprecated;"
+
+ // Medium priority
+ "^::scanf$,strto__,lacks error detection;"
+ "^::fscanf$,strto__,lacks error detection;"
+ "^::sscanf$,strto__,lacks error detection;"
+ "^::vscanf$,strto__,lacks error detection;"
+ "^::vsscanf$,strto__,lacks error detection;"
+
+ "^::atof$,strtod,lacks error detection;"
+ "^::atof_l$,strtod,lacks error detection;"
+ "^::atoi$,strtol,lacks error detection;"
+ "^::atoi_l$,strtol,lacks error detection;"
+ "^::atol$,strtol,lacks error detection;"
+ "^::atol_l$,strtol,lacks error detection;"
+ "^::atoll$,strtoll,lacks error detection;"
+ "^::atoll_l$,strtoll,lacks error detection;"
+ "^::setbuf$,setvbuf,lacks error detection;"
+ "^::setjmp$,,lacks error detection;"
+ "^::sigsetjmp$,,lacks error detection;"
+ "^::longjmp$,,lacks error detection;"
+ "^::siglongjmp$,,lacks error detection;"
+ "^::rewind$,fseek,lacks error detection;"
+
+ // Low priority
+ "^::strtok$,strtok_r,is not thread safe;"
+ "^::strerror$,strerror_r,is not thread safe;"
+ "^::wcstombs$,_FORTIFY_SOURCE,is recommended to have source hardening;"
+ "^::mbstowcs$,_FORTIFY_SOURCE,is recommended to have source hardening;"
+
+ "^::getenv$,,is not thread safe;"
+ "^::mktemp$,mkstemp,is not thread safe;"
+ "^::perror$,,is not thread safe;"
+
+ "^::access$,,is not thread safe;"
+ "^::asctime$,asctime_r,is not thread safe;"
+ "^::atomic_init$,,is not thread safe;"
+ "^::c16rtomb$,,is not thread safe;"
+ "^::c32rtomb$,,is not thread safe;"
+ "^::catgets$,,is not thread safe;"
+ "^::ctermid$,,is not thread safe;"
+ "^::ctime$,cmtime_r,is not thread safe;"
+ "^::dbm_clearerr$,a database client library,is not thread safe;"
+ "^::dbm_close$,a database client library,is not thread safe;"
+ "^::dbm_delete$,a database client library,is not thread safe;"
+ "^::dbm_error$,a database client library,is not thread safe;"
+ "^::dbm_fetch$,a database client library,is not thread safe;"
+ "^::dbm_firstkey$,a database client library,is not thread safe;"
+ "^::dbm_nextkey$,a database client library,is not thread safe;"
+ "^::dbm_open$,a database client library,is not thread safe;"
+ "^::dbm_store$,a database client library,is not thread safe;"
+ "^::dlerror$,,is not thread safe;"
+ "^::drand48$,drand48_r,is not thread safe;"
+ "^::endgrent$,,is not thread safe;"
+ "^::endpwent$,,is not thread safe;"
+ "^::endutxent$,,is not thread safe;"
+ "^::getc_unlocked$,,is not thread safe;"
+ "^::getchar_unlocked$,,is not thread safe;"
+ "^::getdate$,,is not thread safe;"
+ "^::getgrent$,,is not thread safe;"
+ "^::getgrgid$,,is not thread safe;"
+ "^::getgrnam$,,is not thread safe;"
+ "^::gethostent$,,is not thread safe;"
+ "^::getlogin$,,is not thread safe;"
+ "^::getnetbyaddr$,,is not thread safe;"
+ "^::getnetbyname$,,is not thread safe;"
+ "^::getnetent$,,is not thread safe;"
+ "^::getopt$,,is not thread safe;"
+ "^::getprotobyname$,,is not thread safe;"
+ "^::getprotobynumber$,,is not thread safe;"
+ "^::getprotoent$,,is not thread safe;"
+ "^::getpwent$,,is not thread safe;"
+ "^::getpwnam$,,is not thread safe;"
+ "^::getpwuid$,,is not thread safe;"
+ "^::getservbyname$,,is not thread safe;"
+ "^::getservbyport$,,is not thread safe;"
+ "^::getservent$,,is not thread safe;"
+ "^::getutxent$,,is not thread safe;"
+ "^::getutxid$,,is not thread safe;"
+ "^::getutxline$,,is not thread safe;"
+ "^::gmtime$,gmtime_r,is not thread safe;"
+ "^::hcreate$,,is not thread safe;"
+ "^::hdestroy$,,is not thread safe;"
+ "^::hsearch$,,is not thread safe;"
+ "^::inet_ntoa$,,is not thread safe;"
+ "^::l64a$,,is not thread safe;"
+ "^::lgamma$,,is not thread safe;"
+ "^::lgammaf$,,is not thread safe;"
+ "^::lgammal$,,is not thread safe;"
+ "^::localeconv$,,is not thread safe;"
+ "^::localtime$,localtime_r,is not thread safe;"
+ "^::lrand48$,lrand48_r,is not thread safe;"
+ "^::mblen$,,is not thread safe;"
+ "^::mbrto16$,,is not thread safe;"
+ "^::mbrto32$,,is not thread safe;"
+ "^::mbrtowc$,,is not thread safe;"
+ "^::mbsnrtowcs$,,is not thread safe;"
+ "^::mbsrtowcs$,,is not thread safe;"
+ "^::mrand48$,mrand48_r,is not thread safe;"
+ "^::nftw$,,is not thread safe;"
+ "^::ni_langinfo$,,is not thread safe;"
+ "^::ptsname$,,is not thread safe;"
+ "^::putc_unlocked$,,is not thread safe;"
+ "^::putchar_unlocked$,,is not thread safe;"
+ "^::putenv$,,is not thread safe;"
+ "^::pututxline$,,is not thread safe;"
+ "^::rand$,,is not thread safe;"
+ "^::readdir$,,is not thread safe;"
+ "^::setenv$,,is not thread safe;"
+ "^::setgrent$,,is not thread safe;"
+ "^::setkey$,an Ericsson-recommended crypto algorithm,is insecure, "
+ "deprecated, and not thread safe;"
+ "^::setlocale$,,is not thread safe;"
+ "^::setpwent$,,is not thread safe;"
+ "^::setutxent$,,is not thread safe;"
+ "^::srand$,,is not thread safe;"
+ "^::strsignal$,,is not thread safe;"
+ "^::ttyname$,,is not thread safe;"
+ "^::unsetenv$,,is not thread safe;"
+ "^::wctomb$,,is not thread safe;"
+ "^::wcrtomb$,,is not thread safe;"
+ "^::wcsnrtombs$,,is not thread safe;"
+ "^::wcsrtombs$,,is not thread safe;";
return Options;
}
};
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index f967dfabd1c940..94f70c51f00a81 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -192,7 +192,7 @@ Changes in existing checks
- Improved :doc:`bugprone-unsafe-functions
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
- additional functions to match.
+ additional functions to match, including C++ member functions.
- Improved :doc:`bugprone-use-after-move
<clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on
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 fc97d1bc93bc54..b707e471ef7cc4 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,11 +1,16 @@
// 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,,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;^::S::member_match_,,is matched on a C++ class member'}}"
// 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$,,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;^::S::member_match_1$,,is matched on a C++ class member'}}"
void name_match();
void prefix_match();
+struct S {
+ static void member_match_1() {}
+ void member_match_2() {}
+};
+
namespace regex_test {
void name_match();
void prefix_match();
@@ -42,3 +47,19 @@ void f1() {
// 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
}
+
+void f2() {
+ S s;
+
+ S::member_match_1();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+
+ s.member_match_1();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+ // CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
+
+ s.member_match_2();
+ // CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:5: warning: function 'member_match_2' is matched on a C++ class member; it should not be used
+ // no-warning STRICT-REGEX
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/117165
More information about the cfe-commits
mailing list