[clang-tools-extra] 4bcbb3d - [clang-tidy] Add check 'cert-err33-c'.

Balázs Kéri via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 2 03:25:56 PDT 2021


Author: Balázs Kéri
Date: 2021-11-02T11:38:47+01:00
New Revision: 4bcbb3d4d7a821a4ae29ec9333fe9d4c26240286

URL: https://github.com/llvm/llvm-project/commit/4bcbb3d4d7a821a4ae29ec9333fe9d4c26240286
DIFF: https://github.com/llvm/llvm-project/commit/4bcbb3d4d7a821a4ae29ec9333fe9d4c26240286.diff

LOG: [clang-tidy] Add check 'cert-err33-c'.

The CERT rule ERR33-C can be modeled partially by the existing check
'bugprone-unused-return-value'. The existing check is reused with
a fixed set of checked functions.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D112409

Added: 
    clang-tools-extra/docs/clang-tidy/checks/cert-err33-c.rst
    clang-tools-extra/test/clang-tidy/checkers/cert-err33-c.c

Modified: 
    clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
    clang-tools-extra/docs/ReleaseNotes.rst
    clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-return-value.rst
    clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp b/clang-tools-extra/clang-tidy/cert/CERTTidyModule.cpp
index c3cfe12cd8512..3aada6f37f37d 100644
--- 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 @@ class CERTModule : public ClangTidyModule {
         "cert-dcl37-c");
     // ENV
     CheckFactories.registerCheck<CommandProcessorCheck>("cert-env33-c");
+    // ERR
+    CheckFactories.registerCheck<bugprone::UnusedReturnValueCheck>(
+        "cert-err33-c");
+    CheckFactories.registerCheck<StrToNumCheck>("cert-err34-c");
     // EXP
     CheckFactories.registerCheck<bugprone::SuspiciousMemoryComparisonCheck>(
         "cert-exp42-c");
@@ -108,8 +300,6 @@ class CERTModule : public ClangTidyModule {
         "cert-flp37-c");
     // FIO
     CheckFactories.registerCheck<misc::NonCopyableObjectsCheck>("cert-fio38-c");
-    // ERR
-    CheckFactories.registerCheck<StrToNumCheck>("cert-err34-c");
     // MSC
     CheckFactories.registerCheck<LimitedRandomnessCheck>("cert-msc30-c");
     CheckFactories.registerCheck<ProperlySeededRandomGeneratorCheck>(
@@ -131,6 +321,7 @@ class CERTModule : public ClangTidyModule {
     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
index 0ca2c5a5abc6d..5cd131e18ac84 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -103,6 +103,11 @@ New checks
 New check aliases
 ^^^^^^^^^^^^^^^^^
 
+- New alias :doc:`cert-err33-c
+  <clang-tidy/checks/cert-err33-c>` to
+  :doc:`bugprone-unused-return-value
+  <clang-tidy/checks/bugprone-unused-return-value>` was added.
+
 - New alias :doc:`cert-exp42-c
   <clang-tidy/checks/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
index 4cc54ed02d16b..0f33abfb2e318 100644
--- 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 @@ Options
    - ``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 <cert-err33-c.html>`_ 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
index 0000000000000..945bdce6d3296
--- /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 <bugprone-unused-return-value.html>`_
+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
+<https://wiki.sei.cmu.edu/confluence/display/c/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 
diff erentiate 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
index 862817a9b5f68..edb656bc1e48a 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -333,6 +333,7 @@ Clang-Tidy Checks
    `cert-dcl03-c <cert-dcl03-c.html>`_, `misc-static-assert <misc-static-assert.html>`_, "Yes"
    `cert-dcl16-c <cert-dcl16-c.html>`_, `readability-uppercase-literal-suffix <readability-uppercase-literal-suffix.html>`_, "Yes"
    `cert-dcl37-c <cert-dcl37-c.html>`_, `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
+   `cert-err33-c <cert-err33-c.html>`_, `bugprone-unused-return-value <bugprone-unused-return-value.html>`_,
    `cert-dcl51-cpp <cert-dcl51-cpp.html>`_, `bugprone-reserved-identifier <bugprone-reserved-identifier.html>`_, "Yes"
    `cert-dcl54-cpp <cert-dcl54-cpp.html>`_, `misc-new-delete-overloads <misc-new-delete-overloads.html>`_,
    `cert-dcl59-cpp <cert-dcl59-cpp.html>`_, `google-build-namespaces <google-build-namespaces.html>`_,

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
index 0000000000000..b28b54366b5e5
--- /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
+}


        


More information about the cfe-commits mailing list