diff --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp --- a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp +++ b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp @@ -16,6 +16,7 @@ #include "../bugprone/SpuriouslyWakeUpFunctionsCheck.h" #include "../bugprone/SuspiciousMemoryComparisonCheck.h" #include "../bugprone/UnhandledSelfAssignmentCheck.h" +#include "../bugprone/UnusedReturnValueCheck.h" #include "../concurrency/ThreadCanceltypeAsynchronousCheck.h" #include "../google/UnnamedNamespaceInHeaderCheck.h" #include "../misc/NewDeleteOverloadsCheck.h" @@ -39,6 +40,193 @@ #include "ThrownExceptionTypeCheck.h" #include "VariadicFunctionDefCheck.h" +namespace { + +// Checked functions for cert-err33-c. +// The following functions are deliberately excluded because they can be called +// 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;"; + +} // namespace + namespace clang { namespace tidy { namespace cert { @@ -99,6 +287,10 @@ "cert-dcl37-c"); // ENV CheckFactories.registerCheck("cert-env33-c"); + // ERR + CheckFactories.registerCheck( + "cert-err33-c"); + CheckFactories.registerCheck("cert-err34-c"); // EXP CheckFactories.registerCheck( "cert-exp42-c"); @@ -108,8 +300,6 @@ "cert-flp37-c"); // FIO CheckFactories.registerCheck("cert-fio38-c"); - // ERR - CheckFactories.registerCheck("cert-err34-c"); // MSC CheckFactories.registerCheck("cert-msc30-c"); CheckFactories.registerCheck( @@ -131,6 +321,7 @@ ClangTidyOptions Options; ClangTidyOptions::OptionMap &Opts = Options.CheckOptions; Opts["cert-dcl16-c.NewSuffixes"] = "L;LL;LU;LLU"; + Opts["cert-err33-c.CheckedFunctions"] = CertErr33CCheckedFunctions; Opts["cert-oop54-cpp.WarnOnlyIfThisHasSuspiciousField"] = "false"; Opts["cert-str34-c.DiagnoseSignedUnsignedCharComparisons"] = "false"; return Options; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -103,6 +103,11 @@ New check aliases ^^^^^^^^^^^^^^^^^ +- New alias :doc:`cert-err33-c + ` to + :doc:`bugprone-unused-return-value + ` was added. + - New alias :doc:`cert-exp42-c ` to :doc:`bugprone-suspicious-memory-comparison diff --git a/clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst b/clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst --- a/clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst @@ -45,3 +45,6 @@ - ``std::basic_string::empty()`` and ``std::vector::empty()``. Not using the return value often indicates that the programmer confused the function with ``clear()``. + +`cert-err33-c `_ is an alias of this check that checks a +fixed and large set of standard library functions. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst b/clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst new file mode 100644 --- /dev/null +++ b/clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst @@ -0,0 +1,199 @@ +.. title:: clang-tidy - cert-err33-c + +cert-err33-c +============ + +Warns on unused function return values. Many of the standard library functions +return a value that indicates if the call was successful. Ignoring the returned +value can cause unexpected behavior if an error has occured. The following +functions are checked: + +* 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() +* setlocale() +* setvbuf() +* scanf() +* scanf_s() +* 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() +* strtoumax() +* strtoul() +* strtoull() +* 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() +* wcstoumax() +* wcstoul() +* wcstoull() +* wcsxfrm() +* wctob() +* wctrans() +* wctype() +* wmemchr() +* wprintf_s() +* wscanf() +* wscanf_s() + +This check is an alias of check `bugprone-unused-return-value `_ +with a fixed set of functions. + +The check corresponds to a part of CERT C Coding Standard rule `ERR33-C. +Detect and handle standard library errors +`_. +The list of checked functions is taken from the rule, with following exception: + +* The check can not differentiate if a function is called with ``NULL`` + argument. Therefore the following functions are not checked: + ``mblen``, ``mbrlen``, ``mbrtowc``, ``mbtowc``, ``wctomb``, ``wctomb_s`` diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst b/clang-tools-extra/docs/clang-tidy/checks/list.rst --- a/clang-tools-extra/docs/clang-tidy/checks/list.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst @@ -333,6 +333,7 @@ `cert-dcl03-c `_, `misc-static-assert `_, "Yes" `cert-dcl16-c `_, `readability-uppercase-literal-suffix `_, "Yes" `cert-dcl37-c `_, `bugprone-reserved-identifier `_, "Yes" + `cert-err33-c `_, `bugprone-unused-return-value `_, `cert-dcl51-cpp `_, `bugprone-reserved-identifier `_, "Yes" `cert-dcl54-cpp `_, `misc-new-delete-overloads `_, `cert-dcl59-cpp `_, `google-build-namespaces `_, diff --git a/clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c b/clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c new file mode 100644 --- /dev/null +++ b/clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c @@ -0,0 +1,25 @@ +// RUN: %check_clang_tidy %s cert-err33-c %t + +typedef __SIZE_TYPE__ size_t; +void *aligned_alloc(size_t alignment, size_t size); +void test_aligned_alloc() { + aligned_alloc(2, 10); + // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used + // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning +} + +long strtol(const char *restrict nptr, char **restrict endptr, int base); +void test_strtol() { + strtol("123", 0, 10); + // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used + // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning +} + +typedef char wchar_t; +int wscanf_s(const wchar_t *restrict format, ...); +void test_wscanf_s() { + int Val; + wscanf_s("%i", &Val); + // CHECK-NOTES: [[@LINE-1]]:3: warning: the value returned by this function should be used + // CHECK-NOTES: [[@LINE-2]]:3: note: cast the expression to void to silence this warning +}