Skip to content

Commit 8116524

Browse files
DiscookieDiscookie
committed
[clang-tidy] Add C++ member function support to custom bugprone-unsafe-functions matches
1 parent 86fd4d4 commit 8116524

File tree

4 files changed

+210
-12
lines changed

4 files changed

+210
-12
lines changed

‎clang-tools-extra/clang-tidy/bugprone/UnsafeFunctionsCheck.cpp

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,32 @@ void UnsafeFunctionsCheck::registerMatchers(MatchFinder *Finder) {
256256
.bind(CustomFunctionNamesId)))
257257
.bind(DeclRefId),
258258
this);
259+
// C++ member calls do not contain a DeclRefExpr to the function decl.
260+
// Instead, they contain a MemberExpr that refers to the decl.
261+
Finder->addMatcher(memberExpr(member(functionDecl(CustomFunctionsMatcher)
262+
.bind(CustomFunctionNamesId)))
263+
.bind(DeclRefId),
264+
this);
259265
}
260266
}
261267

262268
void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
263-
const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId);
264-
const auto *FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
265-
assert(DeclRef && FuncDecl && "No valid matched node in check()");
269+
const Expr *SourceExpr;
270+
const FunctionDecl *FuncDecl;
271+
272+
if (const auto *DeclRef = Result.Nodes.getNodeAs<DeclRefExpr>(DeclRefId)) {
273+
SourceExpr = DeclRef;
274+
FuncDecl = cast<FunctionDecl>(DeclRef->getDecl());
275+
} else if (const auto *Member =
276+
Result.Nodes.getNodeAs<MemberExpr>(DeclRefId)) {
277+
SourceExpr = Member;
278+
FuncDecl = cast<FunctionDecl>(Member->getMemberDecl());
279+
} else {
280+
llvm_unreachable("No valid matched node in check()");
281+
return;
282+
}
283+
284+
assert(SourceExpr && FuncDecl && "No valid matched node in check()");
266285

267286
// Only one of these are matched at a time.
268287
const auto *AnnexK = Result.Nodes.getNodeAs<FunctionDecl>(
@@ -286,14 +305,15 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
286305
Entry.Reason.empty() ? "is marked as unsafe" : Entry.Reason.c_str();
287306

288307
if (Entry.Replacement.empty()) {
289-
diag(DeclRef->getExprLoc(), "function %0 %1; it should not be used")
308+
diag(SourceExpr->getExprLoc(),
309+
"function %0 %1; it should not be used")
290310
<< FuncDecl << Reason << Entry.Replacement
291-
<< DeclRef->getSourceRange();
311+
<< SourceExpr->getSourceRange();
292312
} else {
293-
diag(DeclRef->getExprLoc(),
313+
diag(SourceExpr->getExprLoc(),
294314
"function %0 %1; '%2' should be used instead")
295315
<< FuncDecl << Reason << Entry.Replacement
296-
<< DeclRef->getSourceRange();
316+
<< SourceExpr->getSourceRange();
297317
}
298318

299319
return;
@@ -323,9 +343,9 @@ void UnsafeFunctionsCheck::check(const MatchFinder::MatchResult &Result) {
323343
if (!ReplacementFunctionName)
324344
return;
325345

326-
diag(DeclRef->getExprLoc(), "function %0 %1; '%2' should be used instead")
346+
diag(SourceExpr->getExprLoc(), "function %0 %1; '%2' should be used instead")
327347
<< FuncDecl << getRationaleFor(FunctionName)
328-
<< ReplacementFunctionName.value() << DeclRef->getSourceRange();
348+
<< ReplacementFunctionName.value() << SourceExpr->getSourceRange();
329349
}
330350

331351
void UnsafeFunctionsCheck::registerPPCallbacks(

‎clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -331,6 +331,10 @@ class CERTModule : public ClangTidyModule {
331331
// STR
332332
CheckFactories.registerCheck<bugprone::SignedCharMisuseCheck>(
333333
"cert-str34-c");
334+
// temp
335+
336+
CheckFactories.registerCheck<bugprone::UnsafeFunctionsCheck>(
337+
"ericsson-unsafe-functions");
334338
}
335339

336340
ClangTidyOptions getModuleOptions() override {
@@ -347,6 +351,159 @@ class CERTModule : public ClangTidyModule {
347351
Opts["cert-err33-c.AllowCastToVoid"] = "true";
348352
Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "false";
349353
Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "false";
354+
Opts["ericsson-unsafe-functions.ReportDefaultFunctions"] = "false";
355+
Opts["ericsson-unsafe-functions.CustomFunctions"] =
356+
// High priority
357+
"^::gets$,fgets,is insecure and deprecated;"
358+
"^::sprintf$,snprintf,is insecure and deprecated;"
359+
"^::strcpy$,strlcpy,is insecure and deprecated;"
360+
"^::strncpy$,strlcpy,is insecure and deprecated;"
361+
"^::vsprintf,vsnprintf,is insecure and deprecated;"
362+
"^::wcscat$,wcslcat,is insecure and deprecated;"
363+
"^::wcscpy$,wcslcpy,is insecure and deprecated;"
364+
"^::wcsncat$,wcslcat,is insecure and deprecated;"
365+
"^::wcsncpy$,wcslcpy,is insecure and deprecated;"
366+
367+
"^::bcopy$,memcpy or memmove,is insecure and deprecated;"
368+
"^::bzero$,memset,is insecure and deprecated;"
369+
"^::index$,strchr,is insecure and deprecated;"
370+
"^::rindex$,strrchr,is insecure and deprecated;"
371+
372+
"^::valloc$,aligned_alloc,is insecure and deprecated;"
373+
374+
"^::tmpnam$,mkstemp,is insecure and deprecated;"
375+
"^::tmpnam_r$,mkstemp,is insecure and deprecated;"
376+
377+
"^::getwd$,getcwd,is insecure and deprecated;"
378+
"^::crypt$,an Ericsson-recommended crypto algorithm,is insecure and "
379+
"deprecated;"
380+
"^::encrypt$,an Ericsson-recommended crypto algorithm,is insecure and "
381+
"deprecated;"
382+
"^::stpcpy$,strlcpy,is insecure and deprecated;"
383+
"^::strcat$,strlcat,is insecure and deprecated;"
384+
"^::strncat$,strlcat,is insecure and deprecated;"
385+
386+
// Medium priority
387+
"^::scanf$,strto__,lacks error detection;"
388+
"^::fscanf$,strto__,lacks error detection;"
389+
"^::sscanf$,strto__,lacks error detection;"
390+
"^::vscanf$,strto__,lacks error detection;"
391+
"^::vsscanf$,strto__,lacks error detection;"
392+
393+
"^::atof$,strtod,lacks error detection;"
394+
"^::atof_l$,strtod,lacks error detection;"
395+
"^::atoi$,strtol,lacks error detection;"
396+
"^::atoi_l$,strtol,lacks error detection;"
397+
"^::atol$,strtol,lacks error detection;"
398+
"^::atol_l$,strtol,lacks error detection;"
399+
"^::atoll$,strtoll,lacks error detection;"
400+
"^::atoll_l$,strtoll,lacks error detection;"
401+
"^::setbuf$,setvbuf,lacks error detection;"
402+
"^::setjmp$,,lacks error detection;"
403+
"^::sigsetjmp$,,lacks error detection;"
404+
"^::longjmp$,,lacks error detection;"
405+
"^::siglongjmp$,,lacks error detection;"
406+
"^::rewind$,fseek,lacks error detection;"
407+
408+
// Low priority
409+
"^::strtok$,strtok_r,is not thread safe;"
410+
"^::strerror$,strerror_r,is not thread safe;"
411+
"^::wcstombs$,_FORTIFY_SOURCE,is recommended to have source hardening;"
412+
"^::mbstowcs$,_FORTIFY_SOURCE,is recommended to have source hardening;"
413+
414+
"^::getenv$,,is not thread safe;"
415+
"^::mktemp$,mkstemp,is not thread safe;"
416+
"^::perror$,,is not thread safe;"
417+
418+
"^::access$,,is not thread safe;"
419+
"^::asctime$,asctime_r,is not thread safe;"
420+
"^::atomic_init$,,is not thread safe;"
421+
"^::c16rtomb$,,is not thread safe;"
422+
"^::c32rtomb$,,is not thread safe;"
423+
"^::catgets$,,is not thread safe;"
424+
"^::ctermid$,,is not thread safe;"
425+
"^::ctime$,cmtime_r,is not thread safe;"
426+
"^::dbm_clearerr$,a database client library,is not thread safe;"
427+
"^::dbm_close$,a database client library,is not thread safe;"
428+
"^::dbm_delete$,a database client library,is not thread safe;"
429+
"^::dbm_error$,a database client library,is not thread safe;"
430+
"^::dbm_fetch$,a database client library,is not thread safe;"
431+
"^::dbm_firstkey$,a database client library,is not thread safe;"
432+
"^::dbm_nextkey$,a database client library,is not thread safe;"
433+
"^::dbm_open$,a database client library,is not thread safe;"
434+
"^::dbm_store$,a database client library,is not thread safe;"
435+
"^::dlerror$,,is not thread safe;"
436+
"^::drand48$,drand48_r,is not thread safe;"
437+
"^::endgrent$,,is not thread safe;"
438+
"^::endpwent$,,is not thread safe;"
439+
"^::endutxent$,,is not thread safe;"
440+
"^::getc_unlocked$,,is not thread safe;"
441+
"^::getchar_unlocked$,,is not thread safe;"
442+
"^::getdate$,,is not thread safe;"
443+
"^::getgrent$,,is not thread safe;"
444+
"^::getgrgid$,,is not thread safe;"
445+
"^::getgrnam$,,is not thread safe;"
446+
"^::gethostent$,,is not thread safe;"
447+
"^::getlogin$,,is not thread safe;"
448+
"^::getnetbyaddr$,,is not thread safe;"
449+
"^::getnetbyname$,,is not thread safe;"
450+
"^::getnetent$,,is not thread safe;"
451+
"^::getopt$,,is not thread safe;"
452+
"^::getprotobyname$,,is not thread safe;"
453+
"^::getprotobynumber$,,is not thread safe;"
454+
"^::getprotoent$,,is not thread safe;"
455+
"^::getpwent$,,is not thread safe;"
456+
"^::getpwnam$,,is not thread safe;"
457+
"^::getpwuid$,,is not thread safe;"
458+
"^::getservbyname$,,is not thread safe;"
459+
"^::getservbyport$,,is not thread safe;"
460+
"^::getservent$,,is not thread safe;"
461+
"^::getutxent$,,is not thread safe;"
462+
"^::getutxid$,,is not thread safe;"
463+
"^::getutxline$,,is not thread safe;"
464+
"^::gmtime$,gmtime_r,is not thread safe;"
465+
"^::hcreate$,,is not thread safe;"
466+
"^::hdestroy$,,is not thread safe;"
467+
"^::hsearch$,,is not thread safe;"
468+
"^::inet_ntoa$,,is not thread safe;"
469+
"^::l64a$,,is not thread safe;"
470+
"^::lgamma$,,is not thread safe;"
471+
"^::lgammaf$,,is not thread safe;"
472+
"^::lgammal$,,is not thread safe;"
473+
"^::localeconv$,,is not thread safe;"
474+
"^::localtime$,localtime_r,is not thread safe;"
475+
"^::lrand48$,lrand48_r,is not thread safe;"
476+
"^::mblen$,,is not thread safe;"
477+
"^::mbrto16$,,is not thread safe;"
478+
"^::mbrto32$,,is not thread safe;"
479+
"^::mbrtowc$,,is not thread safe;"
480+
"^::mbsnrtowcs$,,is not thread safe;"
481+
"^::mbsrtowcs$,,is not thread safe;"
482+
"^::mrand48$,mrand48_r,is not thread safe;"
483+
"^::nftw$,,is not thread safe;"
484+
"^::ni_langinfo$,,is not thread safe;"
485+
"^::ptsname$,,is not thread safe;"
486+
"^::putc_unlocked$,,is not thread safe;"
487+
"^::putchar_unlocked$,,is not thread safe;"
488+
"^::putenv$,,is not thread safe;"
489+
"^::pututxline$,,is not thread safe;"
490+
"^::rand$,,is not thread safe;"
491+
"^::readdir$,,is not thread safe;"
492+
"^::setenv$,,is not thread safe;"
493+
"^::setgrent$,,is not thread safe;"
494+
"^::setkey$,an Ericsson-recommended crypto algorithm,is insecure, "
495+
"deprecated, and not thread safe;"
496+
"^::setlocale$,,is not thread safe;"
497+
"^::setpwent$,,is not thread safe;"
498+
"^::setutxent$,,is not thread safe;"
499+
"^::srand$,,is not thread safe;"
500+
"^::strsignal$,,is not thread safe;"
501+
"^::ttyname$,,is not thread safe;"
502+
"^::unsetenv$,,is not thread safe;"
503+
"^::wctomb$,,is not thread safe;"
504+
"^::wcrtomb$,,is not thread safe;"
505+
"^::wcsnrtombs$,,is not thread safe;"
506+
"^::wcsrtombs$,,is not thread safe;";
350507
return Options;
351508
}
352509
};

‎clang-tools-extra/docs/ReleaseNotes.rst

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -192,7 +192,7 @@ Changes in existing checks
192192

193193
- Improved :doc:`bugprone-unsafe-functions
194194
<clang-tidy/checks/bugprone/unsafe-functions>` check to allow specifying
195-
additional functions to match.
195+
additional functions to match, including C++ member functions.
196196

197197
- Improved :doc:`bugprone-use-after-move
198198
<clang-tidy/checks/bugprone/use-after-move>` to avoid triggering on

‎clang-tools-extra/test/clang-tidy/checkers/bugprone/unsafe-functions-custom-regex.cpp

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
// RUN: %check_clang_tidy -check-suffix=NON-STRICT-REGEX %s bugprone-unsafe-functions %t --\
2-
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '::name_match,replacement,is a qualname match;^::prefix_match,,is matched on qualname prefix'}}"
2+
// 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'}}"
33
// RUN: %check_clang_tidy -check-suffix=STRICT-REGEX %s bugprone-unsafe-functions %t --\
4-
// RUN: -config="{CheckOptions: {bugprone-unsafe-functions.CustomFunctions: '^name_match$,replacement,is matched on function name only;^::prefix_match$,,is a full qualname match'}}"
4+
// 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'}}"
55

66
void name_match();
77
void prefix_match();
88

9+
struct S {
10+
static void member_match_1() {}
11+
void member_match_2() {}
12+
};
13+
914
namespace regex_test {
1015
void name_match();
1116
void prefix_match();
@@ -42,3 +47,19 @@ void f1() {
4247
// CHECK-MESSAGES-NON-STRICT-REGEX: :[[@LINE-1]]:3: warning: function 'prefix_match_regex' is matched on qualname prefix; it should not be used
4348
// no-warning STRICT-REGEX
4449
}
50+
51+
void f2() {
52+
S s;
53+
54+
S::member_match_1();
55+
// 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
56+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:3: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
57+
58+
s.member_match_1();
59+
// 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
60+
// CHECK-MESSAGES-STRICT-REGEX: :[[@LINE-2]]:5: warning: function 'member_match_1' is matched on a C++ class member; it should not be used
61+
62+
s.member_match_2();
63+
// 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
64+
// no-warning STRICT-REGEX
65+
}

0 commit comments

Comments
 (0)