-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[clang-tidy] Fix cert-err33-c
to ignore functions with same prefixes as target
#135160
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Björn Svensson <[email protected]>
…as target PR llvm#82952 introduced regexes matching for CheckedFunctions used by this checker. Fix false positives by adding end-of-string to target regexes. Signed-off-by: Björn Svensson <[email protected]>
@llvm/pr-subscribers-clang-tools-extra @llvm/pr-subscribers-clang-tidy Author: Björn Svensson (bjosv) ChangesPR #82952 introduced regex matching for Add a testcase and fix false positives by adding end-of-string to target regex's. Patch is 24.69 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/135160.diff 2 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index 26befe0de59ae..cc092a9627c5f 100644
--- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
@@ -50,183 +50,183 @@ namespace {
// with NULL argument and in this case the check is not applicable:
// `mblen, mbrlen, mbrtowc, mbtowc, wctomb, wctomb_s`.
// FIXME: The check can be improved to handle such cases.
-const llvm::StringRef CertErr33CCheckedFunctions = "^::aligned_alloc;"
- "^::asctime_s;"
- "^::at_quick_exit;"
- "^::atexit;"
- "^::bsearch;"
- "^::bsearch_s;"
- "^::btowc;"
- "^::c16rtomb;"
- "^::c32rtomb;"
- "^::calloc;"
- "^::clock;"
- "^::cnd_broadcast;"
- "^::cnd_init;"
- "^::cnd_signal;"
- "^::cnd_timedwait;"
- "^::cnd_wait;"
- "^::ctime_s;"
- "^::fclose;"
- "^::fflush;"
- "^::fgetc;"
- "^::fgetpos;"
- "^::fgets;"
- "^::fgetwc;"
- "^::fopen;"
- "^::fopen_s;"
- "^::fprintf;"
- "^::fprintf_s;"
- "^::fputc;"
- "^::fputs;"
- "^::fputwc;"
- "^::fputws;"
- "^::fread;"
- "^::freopen;"
- "^::freopen_s;"
- "^::fscanf;"
- "^::fscanf_s;"
- "^::fseek;"
- "^::fsetpos;"
- "^::ftell;"
- "^::fwprintf;"
- "^::fwprintf_s;"
- "^::fwrite;"
- "^::fwscanf;"
- "^::fwscanf_s;"
- "^::getc;"
- "^::getchar;"
- "^::getenv;"
- "^::getenv_s;"
- "^::gets_s;"
- "^::getwc;"
- "^::getwchar;"
- "^::gmtime;"
- "^::gmtime_s;"
- "^::localtime;"
- "^::localtime_s;"
- "^::malloc;"
- "^::mbrtoc16;"
- "^::mbrtoc32;"
- "^::mbsrtowcs;"
- "^::mbsrtowcs_s;"
- "^::mbstowcs;"
- "^::mbstowcs_s;"
- "^::memchr;"
- "^::mktime;"
- "^::mtx_init;"
- "^::mtx_lock;"
- "^::mtx_timedlock;"
- "^::mtx_trylock;"
- "^::mtx_unlock;"
- "^::printf_s;"
- "^::putc;"
- "^::putwc;"
- "^::raise;"
- "^::realloc;"
- "^::remove;"
- "^::rename;"
- "^::scanf;"
- "^::scanf_s;"
- "^::setlocale;"
- "^::setvbuf;"
- "^::signal;"
- "^::snprintf;"
- "^::snprintf_s;"
- "^::sprintf;"
- "^::sprintf_s;"
- "^::sscanf;"
- "^::sscanf_s;"
- "^::strchr;"
- "^::strerror_s;"
- "^::strftime;"
- "^::strpbrk;"
- "^::strrchr;"
- "^::strstr;"
- "^::strtod;"
- "^::strtof;"
- "^::strtoimax;"
- "^::strtok;"
- "^::strtok_s;"
- "^::strtol;"
- "^::strtold;"
- "^::strtoll;"
- "^::strtoul;"
- "^::strtoull;"
- "^::strtoumax;"
- "^::strxfrm;"
- "^::swprintf;"
- "^::swprintf_s;"
- "^::swscanf;"
- "^::swscanf_s;"
- "^::thrd_create;"
- "^::thrd_detach;"
- "^::thrd_join;"
- "^::thrd_sleep;"
- "^::time;"
- "^::timespec_get;"
- "^::tmpfile;"
- "^::tmpfile_s;"
- "^::tmpnam;"
- "^::tmpnam_s;"
- "^::tss_create;"
- "^::tss_get;"
- "^::tss_set;"
- "^::ungetc;"
- "^::ungetwc;"
- "^::vfprintf;"
- "^::vfprintf_s;"
- "^::vfscanf;"
- "^::vfscanf_s;"
- "^::vfwprintf;"
- "^::vfwprintf_s;"
- "^::vfwscanf;"
- "^::vfwscanf_s;"
- "^::vprintf_s;"
- "^::vscanf;"
- "^::vscanf_s;"
- "^::vsnprintf;"
- "^::vsnprintf_s;"
- "^::vsprintf;"
- "^::vsprintf_s;"
- "^::vsscanf;"
- "^::vsscanf_s;"
- "^::vswprintf;"
- "^::vswprintf_s;"
- "^::vswscanf;"
- "^::vswscanf_s;"
- "^::vwprintf_s;"
- "^::vwscanf;"
- "^::vwscanf_s;"
- "^::wcrtomb;"
- "^::wcschr;"
- "^::wcsftime;"
- "^::wcspbrk;"
- "^::wcsrchr;"
- "^::wcsrtombs;"
- "^::wcsrtombs_s;"
- "^::wcsstr;"
- "^::wcstod;"
- "^::wcstof;"
- "^::wcstoimax;"
- "^::wcstok;"
- "^::wcstok_s;"
- "^::wcstol;"
- "^::wcstold;"
- "^::wcstoll;"
- "^::wcstombs;"
- "^::wcstombs_s;"
- "^::wcstoul;"
- "^::wcstoull;"
- "^::wcstoumax;"
- "^::wcsxfrm;"
- "^::wctob;"
- "^::wctrans;"
- "^::wctype;"
- "^::wmemchr;"
- "^::wprintf_s;"
- "^::wscanf;"
- "^::wscanf_s;";
+const llvm::StringRef CertErr33CCheckedFunctions = "^::aligned_alloc$;"
+ "^::asctime_s$;"
+ "^::at_quick_exit$;"
+ "^::atexit$;"
+ "^::bsearch$;"
+ "^::bsearch_s$;"
+ "^::btowc$;"
+ "^::c16rtomb$;"
+ "^::c32rtomb$;"
+ "^::calloc$;"
+ "^::clock$;"
+ "^::cnd_broadcast$;"
+ "^::cnd_init$;"
+ "^::cnd_signal$;"
+ "^::cnd_timedwait$;"
+ "^::cnd_wait$;"
+ "^::ctime_s$;"
+ "^::fclose$;"
+ "^::fflush$;"
+ "^::fgetc$;"
+ "^::fgetpos$;"
+ "^::fgets$;"
+ "^::fgetwc$;"
+ "^::fopen$;"
+ "^::fopen_s$;"
+ "^::fprintf$;"
+ "^::fprintf_s$;"
+ "^::fputc$;"
+ "^::fputs$;"
+ "^::fputwc$;"
+ "^::fputws$;"
+ "^::fread$;"
+ "^::freopen$;"
+ "^::freopen_s$;"
+ "^::fscanf$;"
+ "^::fscanf_s$;"
+ "^::fseek$;"
+ "^::fsetpos$;"
+ "^::ftell$;"
+ "^::fwprintf$;"
+ "^::fwprintf_s$;"
+ "^::fwrite$;"
+ "^::fwscanf$;"
+ "^::fwscanf_s$;"
+ "^::getc$;"
+ "^::getchar$;"
+ "^::getenv$;"
+ "^::getenv_s$;"
+ "^::gets_s$;"
+ "^::getwc$;"
+ "^::getwchar$;"
+ "^::gmtime$;"
+ "^::gmtime_s$;"
+ "^::localtime$;"
+ "^::localtime_s$;"
+ "^::malloc$;"
+ "^::mbrtoc16$;"
+ "^::mbrtoc32$;"
+ "^::mbsrtowcs$;"
+ "^::mbsrtowcs_s$;"
+ "^::mbstowcs$;"
+ "^::mbstowcs_s$;"
+ "^::memchr$;"
+ "^::mktime$;"
+ "^::mtx_init$;"
+ "^::mtx_lock$;"
+ "^::mtx_timedlock$;"
+ "^::mtx_trylock$;"
+ "^::mtx_unlock$;"
+ "^::printf_s$;"
+ "^::putc$;"
+ "^::putwc$;"
+ "^::raise$;"
+ "^::realloc$;"
+ "^::remove$;"
+ "^::rename$;"
+ "^::scanf$;"
+ "^::scanf_s$;"
+ "^::setlocale$;"
+ "^::setvbuf$;"
+ "^::signal$;"
+ "^::snprintf$;"
+ "^::snprintf_s$;"
+ "^::sprintf$;"
+ "^::sprintf_s$;"
+ "^::sscanf$;"
+ "^::sscanf_s$;"
+ "^::strchr$;"
+ "^::strerror_s$;"
+ "^::strftime$;"
+ "^::strpbrk$;"
+ "^::strrchr$;"
+ "^::strstr$;"
+ "^::strtod$;"
+ "^::strtof$;"
+ "^::strtoimax$;"
+ "^::strtok$;"
+ "^::strtok_s$;"
+ "^::strtol$;"
+ "^::strtold$;"
+ "^::strtoll$;"
+ "^::strtoul$;"
+ "^::strtoull$;"
+ "^::strtoumax$;"
+ "^::strxfrm$;"
+ "^::swprintf$;"
+ "^::swprintf_s$;"
+ "^::swscanf$;"
+ "^::swscanf_s$;"
+ "^::thrd_create$;"
+ "^::thrd_detach$;"
+ "^::thrd_join$;"
+ "^::thrd_sleep$;"
+ "^::time$;"
+ "^::timespec_get$;"
+ "^::tmpfile$;"
+ "^::tmpfile_s$;"
+ "^::tmpnam$;"
+ "^::tmpnam_s$;"
+ "^::tss_create$;"
+ "^::tss_get$;"...
[truncated]
|
"^::wprintf_s;" | ||
"^::wscanf;" | ||
"^::wscanf_s;"; | ||
const llvm::StringRef CertErr33CCheckedFunctions = "^::aligned_alloc$;" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Off topic but this variable should probably be moved to the actual check implementation, instead of being in the CERTTidyModule
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this seems to be an alias to bugprone::UnusedReturnValueCheck
.
I guess cert-err33-c
should be moved to its own implementation similar to hicpp-ignored-remove-result
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, if it's an alias, that explains why. Nevermind then!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should provide an entry in ReleaseNotes.rst
since the change alters behavior of the check (fixes false-positives).
Signed-off-by: Björn Svensson <[email protected]>
Missed that, thanks. Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
…s as target (llvm#135160) PR llvm#82952 introduced regex matching for `CheckedFunctions` in `UnusedReturnValueCheck` which is used by the checker `cert-err33-c`. Add a testcase and fix false positives by adding end-of-string to target regex's. --------- Signed-off-by: Björn Svensson <[email protected]>
PR #82952 introduced regex matching for
CheckedFunctions
inUnusedReturnValueCheck
which is used by the checkercert-err33-c
.Add a testcase and fix false positives by adding end-of-string to target regex's.